-
Notifications
You must be signed in to change notification settings - Fork 245
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
docs: add clarifications to format about fields #1052
Conversation
protos/format.proto
Outdated
// This must be equal in length to the number of columns in the file. The order | ||
// of the ids determines the mapping from field id to column position. |
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.
Is this correct?
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, that's right. So if you already change the format.proto, let's keep a "already assigned max field id" in the manifest? To prevent re-use existing field id if deleted.
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 will do this in a follow-up. #1090
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.
@eddyxu wrote this to make sure I understand how fields work in Lance. LMK if anything here is incorrect.
da0166d
to
f990ec3
Compare
4db96ce
to
4484ff0
Compare
* docs: add clarifications to format * docs: add fragment diagram * docs: clarify field ids are positional
This fills out more details in the existing format.