Skip to content

Conversation

@Danny02
Copy link
Contributor

@Danny02 Danny02 commented Oct 18, 2022

Why:
Javas Optional type can not be used in any meaningful way with the reflection-based schema generation. It is for example not possible to write a generic custom encoding.

How does it help with resolving the issue:
Optional can be used to make fields null safe in their usage.

Side effects:
I think that this change should not have any side effects, because Optional fields could not have been used in the past.

What is the purpose of the change

Implementing AVRO-3644.

Verifying this change

  • Added test that validates that a correct schema is generated
  • Added test that validates that Optional fields can be serialized and deserialized

Documentation

  • This is my big question. Is the reflection serialization documented anywhere?

@github-actions github-actions bot added the Java Pull Requests for Java binding label Oct 18, 2022
@Danny02 Danny02 force-pushed the support-of-util-optional branch from bd363e8 to e4813b4 Compare October 18, 2022 18:03
Copy link
Member

@martin-g martin-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very familiar with the Java SDK but the changes look good to me!

Why:
Javas Optional type can not be used in any meaningful way with the
reflection based schema generation. It is for example not possible
to write a generic custom encoding.

How does it help with resolving the issue:
Optional can be used to make fields null safe in their usage.

Side effects:
I think that this change should not have any side effects, because
Optional fields could not have been used in the past.
@Danny02 Danny02 force-pushed the support-of-util-optional branch from e4813b4 to bd64238 Compare October 18, 2022 20:21
Copy link
Contributor

@RyanSkraba RyanSkraba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work! Any thoughts on UNION order for optionals with @AvroDefault as in #1922?

@RyanSkraba RyanSkraba merged commit 627b005 into apache:master Oct 20, 2022
@Danny02 Danny02 deleted the support-of-util-optional branch October 20, 2022 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants