-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support proto <-> pojo custom type conversion #144
feat: support proto <-> pojo custom type conversion #144
Conversation
dfc456a
to
91996dc
Compare
7de31c6
to
f70a761
Compare
CI is failing TestTypeParser tests, however when I run
|
@JsonDeserialize(as = ImmutableSimpleExtension.FunctionSignatures.class) | ||
@JsonSerialize(as = ImmutableSimpleExtension.FunctionSignatures.class) | ||
@Value.Immutable | ||
public abstract static class FunctionSignatures { | ||
// TODO: Rename to ExtensionSignatures ??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to move away from FunctionSignature as this now has types in it as well, but I'm not sure if ExtensionSignature is a good name either. I'm open to suggestions if anyone has a better alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable
35ffc02
to
b6ecf31
Compare
Running tests using |
@JsonDeserialize(as = ImmutableSimpleExtension.FunctionSignatures.class) | ||
@JsonSerialize(as = ImmutableSimpleExtension.FunctionSignatures.class) | ||
@Value.Immutable | ||
public abstract static class FunctionSignatures { | ||
// TODO: Rename to ExtensionSignatures ??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable
* feat: support proto <-> pojo custom type conversion * test: fix NPE when using Type.NULLABLE/REQUIRED in test --------- Co-authored-by: Jacques Nadeau <jacques@apache.org>
Adds machinery for reading type extensions and handling their conversion in POJOs to proto form and back.
All changes were motivated by the example in TestTypeRoundtrip, which:
The majority of changes stem come from:
io.substrait.type.Type.UserDefined
to the type visitors.There a number of classes w/ TODOs to apply renames and/or moves. I held off on applying them in this PR to keep the diff smaller and also check w/ the community to see if they were reasonable before investing time in them.