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

API, Core: Move schemaID from ViewRepresentation to ViewVersion and make it required #7421

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

amogh-jahagirdar
Copy link
Contributor

Follow up from #6559 (comment) . The conclusion was that it's better to make schema required and part of the ViewVersion since all representations should share the same schema.

@amogh-jahagirdar
Copy link
Contributor Author

I'll wait for #7417 to go in with all the cleanup and then rebase my changes.

Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

mostly looks good to me, will take another look after the rebase

Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

looks good to me, just a nit!

@jackye1995
Copy link
Contributor

Thanks for the quick fix! I will merge this to continue the conversations in #6559

@jackye1995 jackye1995 merged commit 50ca63b into apache:master Apr 25, 2023
34 checks passed
coufon pushed a commit to coufon/iceberg that referenced this pull request Apr 28, 2023
manisin pushed a commit to Snowflake-Labs/iceberg that referenced this pull request May 9, 2023
@nastra nastra added this to Done in View support Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants