Skip to content

Conversation

@aokolnychyi
Copy link
Contributor

This PR is a follow-up to #3373 and throws an exception when table columns conflict with metadata columns during reads.

@github-actions github-actions bot added the spark label Nov 3, 2021
return result;
}

public static void validateMetadataColumnReferences(Schema tableSchema, Schema readSchema) {
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 initially did this for Spark only. It may make sense to move it to DataTableScan in core.

@aokolnychyi
Copy link
Contributor Author

@rdblue
Copy link
Contributor

rdblue commented Nov 3, 2021

Looks good to me. I think this is going to be rare enough that we won't need another solution.

Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

Left a comment but this looks good to me. Thanks @aokolnychyi!

Comment on lines +175 to +178
table.updateSchema()
.addColumn(MetadataColumns.SPEC_ID.name(), Types.IntegerType.get())
.addColumn(MetadataColumns.FILE_PATH.name(), Types.StringType.get())
.commit();
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually, would it make sense to fail here, in the updateSchema pathway?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe. I would like to avoid not allowing these names, but you're right that it would catch the problem earlier.

@rdblue rdblue merged commit 8ef5ed1 into apache:master Nov 5, 2021
KnightChess pushed a commit to KnightChess/iceberg that referenced this pull request Nov 7, 2021
Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 10, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 11, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 11, 2022
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.

3 participants