Strip column mapping metadata when feature is disabled #3688
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johanl-db
approved these changes
Sep 19, 2024
5 tasks
scottsand-db
pushed a commit
that referenced
this pull request
Feb 19, 2025
…s already present (#4167) #### Which Delta project/connector is this regarding? - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description As effect of earlier bugs (e.g. fixed in #3487) there can exists tables where column mapping is disabled, but there is column mapping metadata on the table. Enabling column mapping metadata on such a table could lead to unexpected corruption. Simply stripping such metadata could also lead to curruptions, as the invalid metadata can be already used in other places (e.g. column statistics) via DeltaColumnMapping.getPhysicalName, which returns the name from the metadata even when column mapping is disabled. After #3688 it should no longer be possible to end up with tables having such invalid metadata, so the issue only concerns existing tables created before that fix. To avoid corruption, we want to disallow enabling column mapping on such tables. ## How was this patch tested? Added tests to DeltaColumnMappingSuite. ## Does this PR introduce _any_ user-facing changes? No. We are disallowing an operation on tables that would lead to Delta table corruption on tables that are already in an invalid state entering which is fixed already, so it can only concern old tables in the wild. --------- Co-authored-by: Julek Sompolski <Juliusz Sompolski>
anoopj
pushed a commit
to anoopj/delta
that referenced
this pull request
Feb 24, 2025
…s already present (delta-io#4167) #### Which Delta project/connector is this regarding? - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description As effect of earlier bugs (e.g. fixed in delta-io#3487) there can exists tables where column mapping is disabled, but there is column mapping metadata on the table. Enabling column mapping metadata on such a table could lead to unexpected corruption. Simply stripping such metadata could also lead to curruptions, as the invalid metadata can be already used in other places (e.g. column statistics) via DeltaColumnMapping.getPhysicalName, which returns the name from the metadata even when column mapping is disabled. After delta-io#3688 it should no longer be possible to end up with tables having such invalid metadata, so the issue only concerns existing tables created before that fix. To avoid corruption, we want to disallow enabling column mapping on such tables. ## How was this patch tested? Added tests to DeltaColumnMappingSuite. ## Does this PR introduce _any_ user-facing changes? No. We are disallowing an operation on tables that would lead to Delta table corruption on tables that are already in an invalid state entering which is fixed already, so it can only concern old tables in the wild. --------- Co-authored-by: Julek Sompolski <Juliusz Sompolski>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which Delta project/connector is this regarding?
Description
Transactions might try to create or update the schema of a Delta table with columns that contain column mapping metadata, even when column mapping is not enabled. For example, this can happen when transactions copy the schema from another table without stripping metadata.
To avoid such issues, we automatically strip column mapping metadata when column mapping is disabled. We are doing this only for new tables or for transactions that add column mapping metadata for the first time. If column metadata already exist, we cannot strip them because this would break the table. A usage log is emitted so we can understand the impact on existing tables.
Note that this change covers the cases where txn.updateMetadata is called (the "proper API") and not the cases where a Metadata action is directly committed to the table.
Finally, this commit changes drop column mapping command to check that all column mapping metadata do not exist, and not only physical column name and ID.
How was this patch tested?
Added new UT.
Does this PR introduce any user-facing changes?
No.