This is an automated email from the ASF dual-hosted git repository.

yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git


The following commit(s) were added to refs/heads/main by this push:
     new bae3408207 [#14283] fix(core): Keep the column id when a rename's old 
name differs in case from the stored name (#14286)
bae3408207 is described below

commit bae3408207eb09a8d5fa2412ad9c33fec14afe6f
Author: Qi Yu <[email protected]>
AuthorDate: Tue Oct 27 21:46:59 2026 +0800

    [#14283] fix(core): Keep the column id when a rename's old name differs in 
case from the stored name (#14286)
    
    ### What changes were proposed in this pull request?
    
    When `alterTable` stores a column rename or drop whose name matches no
    stored column exactly, it maps the change to the stored column that
    matches it case-insensitively, if exactly one does
    (`alignNameChangesWithStoredColumns`).
    
    ### Why are the changes needed?
    
    Catalogs such as MySQL match column names case-insensitively but report
    case-sensitive names. A rename of `col` for a column stored as `Col`
    succeeded in the catalog, but Gravitino stored it as a drop of `Col`
    plus an add, so the column got a new id and lost its tags.
    
    Fixes #14283
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    New
    
`TestTableOperationDispatcher.testAlterTableKeepsColumnIdWhenRenameDiffersInCase`
    (fails without the fix) and `testAlignNameChangesWithStoredColumns`.
    `TestTableOperationDispatcher` and `TestTableNormalizeDispatcher` pass.
---
 .../catalog/TableOperationDispatcher.java          | 44 +++++++++++++-
 .../catalog/TestTableOperationDispatcher.java      | 69 ++++++++++++++++++++++
 2 files changed, 112 insertions(+), 1 deletion(-)

diff --git 
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java 
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
index 4b3381ae66..507b3c76c5 100644
--- 
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
+++ 
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
@@ -365,7 +365,10 @@ public class TableOperationDispatcher extends 
OperationDispatcher implements Tab
                             // Update the columns
                             Pair<Boolean, List<ColumnEntity>> 
columnsUpdateResult =
                                 updateColumnsIfNecessary(
-                                    alteredTable, tableEntity, 
catalogResult.columnNameChanges);
+                                    alteredTable,
+                                    tableEntity,
+                                    alignNameChangesWithStoredColumns(
+                                        catalogResult.columnNameChanges, 
tableEntity.columns()));
 
                             return TableEntity.builder()
                                 .withId(tableEntity.id())
@@ -905,6 +908,45 @@ public class TableOperationDispatcher extends 
OperationDispatcher implements Tab
     return originalToCurrent;
   }
 
+  /**
+   * Keys the column renames and drops by the names of the stored columns.
+   *
+   * <p>Some catalogs, such as MySQL, match column names case-insensitively 
while reporting
+   * case-sensitive names, so they accept a change whose column name differs 
in case from the stored
+   * one. Such a name is mapped to the stored column it matches when exactly 
one does, so a rename
+   * keeps the column id instead of being stored as a drop plus an add.
+   *
+   * @param nameChanges the renames and drops returned by {@link 
#resolveColumnNameChanges}
+   * @param storedColumns the columns of the stored table entity, or {@code 
null} if it has none
+   * @return the renames and drops keyed by the stored column names
+   */
+  @VisibleForTesting
+  static Map<String, String> alignNameChangesWithStoredColumns(
+      Map<String, String> nameChanges, @Nullable List<ColumnEntity> 
storedColumns) {
+    if (nameChanges.isEmpty() || storedColumns == null) {
+      return nameChanges;
+    }
+
+    Set<String> storedNames =
+        
storedColumns.stream().map(ColumnEntity::name).collect(Collectors.toSet());
+    Map<String, String> aligned = new HashMap<>();
+    for (Map.Entry<String, String> change : nameChanges.entrySet()) {
+      String name = change.getKey();
+      if (!storedNames.contains(name)) {
+        List<String> matches =
+            storedNames.stream()
+                .filter(stored -> stored.equalsIgnoreCase(change.getKey()))
+                .filter(stored -> !nameChanges.containsKey(stored))
+                .collect(Collectors.toList());
+        if (matches.size() == 1 && !aligned.containsKey(matches.get(0))) {
+          name = matches.get(0);
+        }
+      }
+      aligned.put(name, change.getValue());
+    }
+    return aligned;
+  }
+
   /** Returns the original name of the pre-existing column currently called 
{@code name}. */
   @Nullable
   private static String originalColumnNamed(
diff --git 
a/core/src/test/java/org/apache/gravitino/catalog/TestTableOperationDispatcher.java
 
b/core/src/test/java/org/apache/gravitino/catalog/TestTableOperationDispatcher.java
index 7da2d30644..b9ac73e877 100644
--- 
a/core/src/test/java/org/apache/gravitino/catalog/TestTableOperationDispatcher.java
+++ 
b/core/src/test/java/org/apache/gravitino/catalog/TestTableOperationDispatcher.java
@@ -90,6 +90,7 @@ import org.apache.gravitino.rel.expressions.sorts.SortOrder;
 import org.apache.gravitino.rel.expressions.transforms.Transform;
 import org.apache.gravitino.rel.indexes.Indexes;
 import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.storage.RandomIdGenerator;
 import org.apache.gravitino.storage.relational.po.ColumnPO;
 import org.apache.gravitino.storage.relational.po.TablePO;
 import org.apache.gravitino.storage.relational.utils.POConverters;
@@ -1535,6 +1536,74 @@ public class TestTableOperationDispatcher extends 
TestOperationDispatcher {
     Assertions.assertEquals(2, afterRename.size());
   }
 
+  @Test
+  public void testAlterTableKeepsColumnIdWhenRenameDiffersInCase() throws 
IOException {
+    Namespace tableNs = Namespace.of(metalake, catalog, 
"schema_column_rename_case");
+    Map<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
+    
schemaOperationDispatcher.createSchema(NameIdentifier.of(tableNs.levels()), 
"comment", props);
+    NameIdentifier tableIdent = NameIdentifier.of(tableNs, 
"table_column_rename_case");
+    Column[] columns =
+        new Column[] {
+          TestColumn.builder()
+              .withName("col")
+              .withPosition(0)
+              .withType(Types.IntegerType.get())
+              .build()
+        };
+    tableOperationDispatcher.createTable(tableIdent, columns, "comment", 
props, new Transform[0]);
+    // A catalog that matches column names case-insensitively, such as MySQL, 
accepts "col" for a
+    // column stored as "Col".
+    entityStore.update(
+        tableIdent,
+        TableEntity.class,
+        TABLE,
+        (TableEntity old) -> renameStoredColumn(old, "col", "Col"));
+    long colId = columnIds(tableIdent).get("Col");
+
+    tableOperationDispatcher.alterTable(
+        tableIdent, TableChange.renameColumn(new String[] {"col"}, "renamed"));
+
+    Map<String, Long> ids = columnIds(tableIdent);
+    Assertions.assertEquals(ImmutableMap.of("renamed", colId), ids);
+  }
+
+  @Test
+  public void testAlignNameChangesWithStoredColumns() {
+    List<ColumnEntity> stored =
+        Arrays.asList(storedColumn("Col"), storedColumn("Other"), 
storedColumn("dup"));
+
+    Map<String, String> changes = new HashMap<>();
+    changes.put("col", "renamed");
+    changes.put("OTHER", null);
+    Map<String, String> expected = new HashMap<>();
+    expected.put("Col", "renamed");
+    expected.put("Other", null);
+    Assertions.assertEquals(
+        expected, 
TableOperationDispatcher.alignNameChangesWithStoredColumns(changes, stored));
+
+    // An exact match wins, and a name matching several stored columns is left 
as it is.
+    List<ColumnEntity> ambiguous =
+        Arrays.asList(
+            storedColumn("a"), storedColumn("A"), storedColumn("Dup"), 
storedColumn("DUP"));
+    Map<String, String> exactAndAmbiguous = ImmutableMap.of("a", "b", "dup", 
"c");
+    Assertions.assertEquals(
+        exactAndAmbiguous,
+        
TableOperationDispatcher.alignNameChangesWithStoredColumns(exactAndAmbiguous, 
ambiguous));
+  }
+
+  private static ColumnEntity storedColumn(String name) {
+    return ColumnEntity.builder()
+        .withId(RandomIdGenerator.INSTANCE.nextId())
+        .withName(name)
+        .withPosition(0)
+        .withDataType(Types.IntegerType.get())
+        .withNullable(true)
+        .withAutoIncrement(false)
+        .withAuditInfo(
+            
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
+        .build();
+  }
+
   @Test
   public void testResolveColumnNameChanges() {
     Assertions.assertEquals(

Reply via email to