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

Spec: remove the JSON spec for content file and file scan task sections #9771

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

stevenzwu
Copy link
Contributor

@stevenzwu stevenzwu commented Feb 21, 2024

replaced with link to Java implementation and note for additional context.

See the discussion thread on dev mailing list.
https://lists.apache.org/thread/2ty27yx4q0zlqd5h71cyyhb5k47yf9bv

@github-actions github-actions bot added the Specification Issues that may introduce spec changes. label Feb 21, 2024
Copy link
Contributor

@pvary pvary left a comment

Choose a reason for hiding this comment

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

Approved, as discussed on the dev list.
@stevenzwu: could you please link the dev list discussion to the PR for future reference?

@jackye1995
Copy link
Contributor

Hey I am just back from my leave, sorry for the late review. Why not just remove these sections completely? The JSON serialization of ContentFile and FileScanTask needs to evolve in a backwards compatible way I think that is for sure, but it is at this point not something relevant with the core table spec, but more of an engine feature implementation detail right?

They shouldn't be part of the core table spec although the JSON serializer is valuable for FileScanTask serialization. See discussion thread for more context: https://lists.apache.org/thread/2ty27yx4q0zlqd5h71cyyhb5k47yf9bv
@stevenzwu
Copy link
Contributor Author

Hey I am just back from my leave, sorry for the late review. Why not just remove these sections completely? The JSON serialization of ContentFile and FileScanTask needs to evolve in a backwards compatible way I think that is for sure, but it is at this point not something relevant with the core table spec, but more of an engine feature implementation detail right?

done. let me also raise a vote thread for spec change.

@stevenzwu stevenzwu merged commit ab580b9 into apache:main Jul 16, 2024
2 checks passed
@stevenzwu stevenzwu deleted the spec branch July 16, 2024 04:02
jasonf20 pushed a commit to jasonf20/iceberg that referenced this pull request Aug 4, 2024
…ns. (apache#9771)

They shouldn't be part of the core table spec although the JSON serializer is valuable for FileScanTask serialization. See discussion thread for more context: https://lists.apache.org/thread/2ty27yx4q0zlqd5h71cyyhb5k47yf9bv
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
…ns. (apache#9771)

They shouldn't be part of the core table spec although the JSON serializer is valuable for FileScanTask serialization. See discussion thread for more context: https://lists.apache.org/thread/2ty27yx4q0zlqd5h71cyyhb5k47yf9bv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Specification Issues that may introduce spec changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants