fix(spec): rewrite field-scoped options on column rename - #635
Open
u70b3 wants to merge 1 commit into
Open
Conversation
RenameColumn only rewrote fields.<col>.aggregate-function and .list-agg-delimiter keys, stranding every other field-scoped option on the old column name. Port the remaining cases of Java SchemaManager.applyRenameColumnsToOptions: - case 2: also rename ignore-retract, distinct, and the map shared-shredding keys (map.storage-layout, map.shared-shredding.max-columns, map.shared-shredding.column-placement-policy) - case 3: rename matching entries in both the key's field list and the value of fields.<cols>.sequence-group / .nested-key options Linked issue: close apache#383 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Purpose
Linked issue: close #383
RenameColumnpreviously rewrote onlyfields.<col>.aggregate-function/.list-agg-delimiterkeys (case 2 subset). Every other field-scoped option stayed on the old column name and was silently persisted: partial-updatefields.<cols>.sequence-group(both key and value),fields.<col>.nested-key,ignore-retract/distinct, and the map shared-shredding keys — so a rename could silently change merge semantics or revert a shredded MAP column to the default layout.This ports the remaining case 2 suffixes and case 3 of Java
SchemaManager.applyRenameColumnsToOptions.Brief change log
fields.<col>.<suffix>, value has no column names): the renameable suffix list grows from 2 to Java's full 7 — addsignore-retract,distinct,map.storage-layout,map.shared-shredding.max-columns,map.shared-shredding.column-placement-policy.fields.<col,col,...>.<suffix>forsequence-group/nested-key): rename exact, comma-separated matches in BOTH the key's field list and the value, mirroring Java'sapplyNotNestedColumnRename(whole-name match only — renamingpricedoes not touchprice2, order preserved).remove_field_scoped_options) shares the case-2 suffix table, so dropping a column now also cleans up the newly covered keys.Tests
cargo test -p paimon --lib spec::— 454 passed, including:abuntouched when renaminga); case-3 rewrites key + value, unrelated options untouched.apply_changestests: rename of a sequence-group key field and value field (fields.version,source_order.sequence-group=price,quantity->fields.version,order_seq.sequence-group=amount,quantity),nested-keykey + value, remaining case-2 suffixes incl. a MAP column, and whole-name matching.test_rename_column_rewrites_field_scoped_agg_optionsunchanged and passing.cargo test -p paimon --lib— 2054 passed.cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warningsclean.API and Format
No public API or storage format change; option keys are rewritten in place during schema evolution, matching Java metadata layout.
Documentation
No documentation change; code comments updated to reference the mirrored Java method.
🤖 Generated with Claude Code
Assumptions / notes for reviewers (per the repo AI-policy):
map.*suffixes were added to Java's case-2 list after the issue text was written; included because Rust implements map shared-shredding (fields.<col>.map.storage-layoutinarrow/shredding/map.rs), so renaming a shredded MAP column would otherwise strand the layout option.sequence-group/nested-key/ignore-retract/distincton tables withoutmerge-engine, because Rust's create-time engine validation rejects those options (they arrive via Java-written metadata). Related pre-existing limitation, out of scope here:apply_changesre-runs create-mode validation, so ANY schema change on a Java-written partial-update table with sequence-group options is currently rejected — relaxing alter-time validation could be a follow-up issue.a->bplusb->cin one ALTER) apply sequentially per change, consistent with the existing case-1 handling (bucket-key/sequence.field); Java applies its rename map in one pass. This pathological case produces a different end state between the two implementations, same as case 1 today.