Conversation
| public static NestedField required(int id, String name, Type type, String doc, | ||
| Object initialDefault, Object writeDefault) { |
There was a problem hiding this comment.
Is it acceptable to always expect doc if we need to declare a default value?
There was a problem hiding this comment.
I don't see a way around this so it should be okay.
| return this; | ||
| } | ||
| return new NestedField(true, id, name, type, doc); | ||
| return new NestedField(true, id, name, type, doc, initialDefault, writeDefault); |
There was a problem hiding this comment.
Should the API always expect intialDefault and writeDefault? Can there be an option to provide initialDefault only, and set writeDefault to the same value?
There was a problem hiding this comment.
If possible, I'd really like to hide the complexity of both defaults as you're suggesting. Unfortunately, I don't see a good way to do that.
There was a problem hiding this comment.
+1, but I think we could handle that in the table API.
|
@wmoustafa, @rzhang10, overall this looks fine to me. The only issue I have is that if we were to merge this now, we'd be updating a public API when there is no implementation behind it. I think we should hold off on merging this until it is time to expose the ability to set these. Before we do this, there are related PRs that we can get done:
|
|
Hi @rdblue @wmoustafa @rzhang10 I am happy to contribute towards any pending or todo items. Please let me know how I can help. Thanks! |
|
Hey guys, I have some questions about the API change here.
|
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
5 participants
This PR enhances the API of
NestedFieldto include default values, they are represented as JavaObjectin memory, this PR is the first PR to implement the spec PR #4301 .The API changes will be the basis for subsequent PRs to implement the java in-mem to/from json round trip serialization and deserialization, and to support different engines to read/write default values in various formats, according to the semantics in the spec.
@rdblue @wmoustafa