Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Draft catalog flatbuffers schema #82
Draft catalog flatbuffers schema #82
Changes from all commits
38c9602
5a41824
a45e13c
334a139
f5f6ea9
bacadc5
86f3584
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Flatbuffers allows you to explicitly assign integer IDs to fields ala protobufs, with the
id
attribute. Can that work for our field ordinal value?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 didn't think of that. The ordinal was just mean to keep the order. I'm not sure if anyone planned to assign ids for access - we could use these ordinals for that as well.
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.
Any field in a flatbuffers table is nullable by default (absence is denoted by an offset of 0 for that field in the object's vtbl).
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.
Yes, but this was meant to track whether the field should accept null or not. In tables, you can treat null value as missing, but in structs we'd need some other approach.
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.
Right, in that case I think we'd need to track a user-defined "missing" sentinel value in our schema representation. Or just say that all fields in a struct are required as flatbuffers does.
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.
Actually, missing may not work for tables either. We've had an early discussion where we've said that we'd need to avoid missing values in production, to allow in-place updates. flatc has an option to allow default values to be written in the serialization. The null discussion may need to wait until after Q2, to be fully addressed.