From baf9ed58ce0b33d78e19ffa186ac314b9d4149d2 Mon Sep 17 00:00:00 2001 From: Rishi Reddy Bokka Date: Fri, 9 May 2025 15:44:20 -0400 Subject: [PATCH 1/2] Added a fix for column rename in IcebergSchemaSync --- .../xtable/iceberg/IcebergSchemaSync.java | 22 ++++++++++----- .../xtable/iceberg/TestIcebergSchemaSync.java | 27 +++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java index 800938cb4..fe4ce9d4d 100644 --- a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java +++ b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java @@ -74,12 +74,12 @@ private static Map> addUpdates( UpdateSchema updateSchema, String parentPath) { Map> updates = new HashMap<>(); - Set allColumnNames = new HashSet<>(); - current.fields().stream().map(Types.NestedField::name).forEach(allColumnNames::add); - latest.fields().stream().map(Types.NestedField::name).forEach(allColumnNames::add); - for (String columnName : allColumnNames) { - Types.NestedField latestColumn = latest.field(columnName); - Types.NestedField currentColumn = current.field(columnName); + Set allColumnFieldIds = new HashSet<>(); + current.fields().stream().map(Types.NestedField::fieldId).forEach(allColumnFieldIds::add); + latest.fields().stream().map(Types.NestedField::fieldId).forEach(allColumnFieldIds::add); + for (int columnFieldId : allColumnFieldIds) { + Types.NestedField latestColumn = latest.field(columnFieldId); + Types.NestedField currentColumn = current.field(columnFieldId); if (currentColumn == null) { // add a new column if (latestColumn.isOptional()) { @@ -99,7 +99,9 @@ private static Map> addUpdates( // drop the existing column, use fieldId 0 to perform deletes first updates.put( 0, - () -> updateSchema.deleteColumn(constructFullyQualifiedName(columnName, parentPath))); + () -> + updateSchema.deleteColumn( + constructFullyQualifiedName(currentColumn.name(), parentPath))); } else { updates.putAll(updateColumn(latestColumn, currentColumn, updateSchema, parentPath)); } @@ -114,6 +116,12 @@ private static Map> updateColumn( String parentPath) { Map> updates = new HashMap<>(); if (!latestColumn.equals(currentColumn)) { + // update the name of the column + if (!latestColumn.name().equals(currentColumn.name())) { + updates.put( + latestColumn.fieldId(), + () -> updateSchema.renameColumn(currentColumn.name(), latestColumn.name())); + } // update the type of the column if (latestColumn.type().isPrimitiveType() && !latestColumn.type().equals(currentColumn.type())) { diff --git a/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSchemaSync.java b/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSchemaSync.java index 98254591e..c240dc5f8 100644 --- a/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSchemaSync.java +++ b/xtable-core/src/test/java/org/apache/xtable/iceberg/TestIcebergSchemaSync.java @@ -335,6 +335,33 @@ void testAddMapFieldComment() { verify(mockUpdateSchema).commit(); } + @Test + public void testUpdateColumnName() { + UpdateSchema mockUpdateSchema = Mockito.mock(UpdateSchema.class); + when(mockTransaction.updateSchema()).thenReturn(mockUpdateSchema); + schemaSync.sync(SCHEMA, updateColumnName(2), mockTransaction); + + verify(mockUpdateSchema).renameColumn(SCHEMA.findColumnName(2), "updateColumnName"); + verify(mockUpdateSchema).commit(); + } + + private Schema updateColumnName(int fieldId) { + List fields = new ArrayList<>(); + for (Types.NestedField existingField : SCHEMA.columns()) { + if (existingField.fieldId() == fieldId) { + fields.add( + Types.NestedField.of( + existingField.fieldId(), + existingField.isOptional(), + "updateColumnName", + existingField.type())); + } else { + fields.add(existingField); + } + } + return new Schema(fields); + } + private Schema addColumnToDefault(Schema schema, Types.NestedField field, Integer parentId) { List fields = new ArrayList<>(); for (Types.NestedField existingField : schema.columns()) { From 606c8635229a62524dfeb64e27101c7c0876ba14 Mon Sep 17 00:00:00 2001 From: Rishi Reddy Bokka Date: Fri, 9 May 2025 19:25:20 -0400 Subject: [PATCH 2/2] Added a fix for column rename in IcebergSchemaSync --- .../org/apache/xtable/iceberg/IcebergSchemaSync.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java index fe4ce9d4d..aef645be1 100644 --- a/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java +++ b/xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergSchemaSync.java @@ -116,12 +116,6 @@ private static Map> updateColumn( String parentPath) { Map> updates = new HashMap<>(); if (!latestColumn.equals(currentColumn)) { - // update the name of the column - if (!latestColumn.name().equals(currentColumn.name())) { - updates.put( - latestColumn.fieldId(), - () -> updateSchema.renameColumn(currentColumn.name(), latestColumn.name())); - } // update the type of the column if (latestColumn.type().isPrimitiveType() && !latestColumn.type().equals(currentColumn.type())) { @@ -131,6 +125,12 @@ private static Map> updateColumn( updateSchema.updateColumn( latestColumn.name(), latestColumn.type().asPrimitiveType())); } + // update the name of the column + if (!latestColumn.name().equals(currentColumn.name())) { + updates.put( + latestColumn.fieldId(), + () -> updateSchema.renameColumn(currentColumn.name(), latestColumn.name())); + } // update whether the column is required if (latestColumn.isOptional() != currentColumn.isOptional()) { if (latestColumn.isOptional()) {