Repository navigation
Check partition names against the current schema when adding a spec - #4266
Open
Rahul-cs50 wants to merge 1 commit into
Open
Rahul-cs50 wants to merge 1 commit into
Rahul-cs50 wants to merge 1 commit into
Conversation
UpdateSpec checks new partition names against the schema it was loaded with. If another writer added a column with the same name in between, the commit still went through and wrote metadata Java refuses to load. Check the names again when the spec is applied to the current metadata. Closes apache#4198
This branch has not been deployed
This file contains hidden or 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
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.
Closes #4198
Rationale for this change
UpdateSpecchecks each new partition name against the schema it was loaded with, using the check from #2305. Its commit asserts only the last assigned partition id and the default spec id. Suppose another writer adds a columnxbin between. A stale handle can then add a partition field namedxb, and the commit goes through. Java then refuses to load that metadata (Cannot create identity partition sourced from different field in schema: xb).This change runs
validate_partition_nameagain whenAddPartitionSpecUpdateis applied, against the schema of the metadata the spec is being added to. On a catalog that applies updates to the latest metadata at commit time, the staleupdate_specnow raises the sameValueErrorit raises without the concurrent change. The schema-side check inUpdateSchemaalready validates every partition spec in this way.The issue also mentions the opposite order: a stale
update_schema().add_column("xb")after another writer added partition fieldxb. That is the same problem, but on the schema update path. I kept it out of this PR so this one stays small, and can follow up if wanted.Are these changes tested?
Yes.
test_concurrent_update_spec_checks_names_against_current_schemaintests/catalog/test_catalog_behaviors.pyfollows the issue's steps:xb.xb.ValueError, and checks that the table is still unpartitioned.It fails on
main(no error is raised) and passes with this change, for all catalog fixtures. Unit tests (tests/without integration and cloud suites): 4501 passed; the 4 failures are Kerberos tests intests/catalog/test_hive.pythat fail the same way onmainin my environment.ruff checkandruff format --checkare clean.Are there any user-facing changes?
A stale
update_spec()that adds a partition field whose name now clashes with a column in the current schema raisesValueErrorat commit. Before, it wrote metadata that Java refuses to load.