From 9e60b006b5f5194235d0d0d2f53ba04180fd7720 Mon Sep 17 00:00:00 2001 From: "wenchao.wu" Date: Mon, 8 Jun 2026 18:47:23 +0800 Subject: [PATCH] [core] Fix false-positive immutability check for normalized options like primary-key and partition. --- .../apache/paimon/schema/SchemaManager.java | 44 ++++++++++++++++--- .../paimon/table/AbstractFileStoreTable.java | 9 ++-- .../paimon/schema/SchemaManagerTest.java | 25 +++++++++++ 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java b/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java index 905f30704864..92a090848224 100644 --- a/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java +++ b/paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java @@ -321,14 +321,18 @@ public static TableSchema generateTableSchema( for (SchemaChange change : changes) { if (change instanceof SetOption) { SetOption setOption = (SetOption) change; - if (hasSnapshots.get()) { - checkAlterTableOption( - oldOptions, - setOption.key(), - oldOptions.get(setOption.key()), - setOption.value()); + String oldValue = oldOptions.get(setOption.key()); + String newValue = setOption.value(); + boolean unchanged = + Objects.equals(oldValue, newValue) + || isUnchangedNormalizedKey( + setOption.key(), oldValue, newValue, oldTableSchema); + if (hasSnapshots.get() && !unchanged) { + checkAlterTableOption(oldOptions, setOption.key(), oldValue, newValue); + } + if (!unchanged) { + newOptions.put(setOption.key(), setOption.value()); } - newOptions.put(setOption.key(), setOption.value()); } else if (change instanceof RemoveOption) { RemoveOption removeOption = (RemoveOption) change; if (hasSnapshots.get()) { @@ -1251,6 +1255,32 @@ public void rollbackTo( } } + /** + * Checks whether a key whose old value is null actually hasn't changed. This handles keys like + * 'primary-key' and 'partition' that are stripped from options during schema normalization and + * stored in dedicated schema fields instead. + */ + public static boolean isUnchangedNormalizedKey( + String key, @Nullable String oldValue, String newValue, TableSchema tableSchema) { + if (oldValue != null) { + return false; + } + if (CoreOptions.PRIMARY_KEY.key().equals(key)) { + return normalizeKeyList(newValue).equals(tableSchema.primaryKeys()); + } + if (CoreOptions.PARTITION.key().equals(key)) { + return normalizeKeyList(newValue).equals(tableSchema.partitionKeys()); + } + return false; + } + + private static List normalizeKeyList(String value) { + return Arrays.stream(value.split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toList()); + } + public static void checkAlterTableOption( Map options, String key, @Nullable String oldValue, String newValue) { if (CoreOptions.IMMUTABLE_OPTIONS.contains(key)) { diff --git a/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java b/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java index 0249a8e6b397..a35167d88498 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java @@ -337,7 +337,9 @@ private void checkImmutability(Map dynamicOptions) { dynamicOptions.forEach( (k, newValue) -> { String oldValue = oldOptions.get(k); - if (!Objects.equals(oldValue, newValue)) { + if (!Objects.equals(oldValue, newValue) + && !SchemaManager.isUnchangedNormalizedKey( + k, oldValue, newValue, tableSchema)) { SchemaManager.checkAlterTableOption(oldOptions, k, oldValue, newValue); } }); @@ -347,12 +349,13 @@ protected FileStoreTable copyInternal( Map dynamicOptions, boolean tryTimeTravel) { Map options = new HashMap<>(tableSchema.options()); - // merge non-null dynamic options into schema.options + // merge dynamic options into schema.options dynamicOptions.forEach( (k, v) -> { if (v == null) { options.remove(k); - } else { + } else if (!SchemaManager.isUnchangedNormalizedKey( + k, tableSchema.options().get(k), v, tableSchema)) { options.put(k, v); } }); diff --git a/paimon-core/src/test/java/org/apache/paimon/schema/SchemaManagerTest.java b/paimon-core/src/test/java/org/apache/paimon/schema/SchemaManagerTest.java index 9a15cab36159..8b80627185a2 100644 --- a/paimon-core/src/test/java/org/apache/paimon/schema/SchemaManagerTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/schema/SchemaManagerTest.java @@ -460,6 +460,31 @@ public void testAlterImmutableOptionsOnEmptyTable() throws Exception { .hasMessage("Change 'merge-engine' is not supported yet."); } + @Test + public void testCopyWithPrimaryKeyInOptions() throws Exception { + // Table created with primary-key in options — normalizePrimaryKeys strips it from the + // options map and stores it in the dedicated primaryKeys field. When the same value + // reappears in dynamicOptions (e.g. Spark 4.x merging Table.properties() into scan + // options), the immutability check should recognize it hasn't actually changed. + Map tableOptions = new HashMap<>(); + tableOptions.put("primary-key", "f0,f1"); + Schema schema = + new Schema( + rowType.getFields(), + Collections.emptyList(), + Collections.emptyList(), + tableOptions, + ""); + Path tableRoot = new Path(tempDir.toString(), "table"); + SchemaManager manager = new SchemaManager(LocalFileIO.create(), tableRoot); + manager.createTable(schema); + + FileStoreTable table = FileStoreTableFactory.create(LocalFileIO.create(), tableRoot); + FileStoreTable copied = table.copy(Collections.singletonMap("primary-key", "f0,f1")); + assertThatCode(() -> copied.schema().toSchema()).doesNotThrowAnyException(); + assertThat(copied.schema().options()).doesNotContainKey("primary-key"); + } + @Test public void testDropPrimaryKeyOnEmptyTable() throws Exception { Path tableRoot = new Path(tempDir.toString(), "table");