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

Doc: improve description for sort order fields in spec #2201

Closed
wants to merge 1 commit into from

Conversation

jackye1995
Copy link
Contributor

This PR is for the followup in #2055 (comment)

@github-actions github-actions bot added the docs label Feb 2, 2021
| _optional_ | _required_ | **`sort-orders`**| A list of sort orders, stored as full sort order objects. |
| _optional_ | _required_ | **`default-sort-order-id`**| Default sort order id of the table. Note that this could be used by writers, but is not used when reading because reads use the specs stored in manifest files. |
| _optional_ | _required_ | **`sort-orders`**| A list of sort orders, stored as full sort order objects. Note that this field is optional when v2 is reading because v1 metadata is still valid, but a v2 writer must always write this field. |
| _optional_ | _required_ | **`default-sort-order-id`**| Default sort order id of the table. Note that this could be used by writers, but is not used when reading because reads use the specs stored in manifest files. Also note that this field is optional when v2 is reading because v1 metadata is still valid, but a v2 writer must always write this field. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this information should be covered in a more general section about how to interpret the optional and required columns. V2 readers should be able to read v1 metadata, which means anything that is omitted or missing in v1 is optional when reading for v2. The required here always refers to the writer, and we don't need to include an extra sentence explaining that in each row.

@jackye1995 jackye1995 closed this Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants