Draft catalog flatbuffers schema #82
Conversation
| enabled: bool; | ||
|
|
||
| // Will require some flatbuffers modification to support. | ||
| nullable: bool; |
There was a problem hiding this comment.
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.
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.
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.
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.
| // Monotonically assigned identifier, to maintain the order of fields in flatbuffers schema. | ||
| // | ||
| // Do we need to support more than 65k fields? | ||
| ordering_id: ushort; |
There was a problem hiding this comment.
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.
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.
| ordering_id: ushort; | ||
|
|
||
| // For deprecating fields. | ||
| enabled: bool; |
There was a problem hiding this comment.
Flatbuffers has a deprecated attribute which we should use.
There was a problem hiding this comment.
This is exactly what this would track. Whether this field should be marked as deprecated in the schema or not. Or were you suggesting that we call it 'deprecated' as well?
There was a problem hiding this comment.
No, just wanted to confirm that this tracks the flatbuffers deprecated attribute, although I would like to know why you diverged from their terminology.
There was a problem hiding this comment.
I can't remember - something made me want to invert the name, but I don't remember now. Might have been to avoid negated logic, because 'enabled = true' reads better than 'deprecated = false'. Let me think a bit about it.
There was a problem hiding this comment.
I renamed it to deprecated. I also renamed a couple of other fields (type->category, count->repeated_count) for better clarity.
Creating this draft catalog flatbuffers schema, for sharing ideas.
Currently, this mainly covers type and table metadata. This metadata is meant to be sufficient to construct a flatbuffers schema that can then be processed to generate accessors, etc.
See comments for details.