Skip to content
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

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

wjones127
Copy link
Contributor

This fills out more details in the existing format.

Comment on lines 144 to 145
// 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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

@wjones127 wjones127 left a 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.

@wjones127 wjones127 changed the title docs: add clarifications to format docs: add clarifications to format about fields Jul 10, 2023
@wjones127 wjones127 requested a review from eddyxu July 10, 2023 22:40
@wjones127 wjones127 force-pushed the wjones127/format-docs-more branch from da0166d to f990ec3 Compare July 10, 2023 23:48
@wjones127 wjones127 force-pushed the wjones127/format-docs-more branch from 4db96ce to 4484ff0 Compare July 24, 2023 20:08
@wjones127 wjones127 merged commit 2f99f7b into main Jul 24, 2023
@wjones127 wjones127 deleted the wjones127/format-docs-more branch July 24, 2023 21:03
eddyxu pushed a commit that referenced this pull request Jul 25, 2023
* docs: add clarifications to format

* docs: add fragment diagram

* docs: clarify field ids are positional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants