From b3d4490cc2400b7b003068bd819a65ad08745088 Mon Sep 17 00:00:00 2001 From: "wangzixuan.wzxuan" Date: Wed, 24 Aug 2022 19:47:20 +0800 Subject: [PATCH 1/2] [HUDI-4706] Fix InternalSchemaChangeApplier#applyAddChange error to add nest type --- .../internal/schema/action/InternalSchemaChangeApplier.java | 3 ++- .../hudi/internal/schema/action/TableChangesHelper.java | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/InternalSchemaChangeApplier.java b/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/InternalSchemaChangeApplier.java index c84d2fa23972a..36aac462a137e 100644 --- a/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/InternalSchemaChangeApplier.java +++ b/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/InternalSchemaChangeApplier.java @@ -51,7 +51,8 @@ public InternalSchema applyAddChange( TableChange.ColumnPositionChange.ColumnPositionType positionType) { TableChanges.ColumnAddChange add = TableChanges.ColumnAddChange.get(latestSchema); String parentName = TableChangesHelper.getParentName(colName); - add.addColumns(parentName, colName, colType, doc); + String leafName = TableChangesHelper.getLeafName(colName); + add.addColumns(parentName, leafName, colType, doc); if (positionType != null) { switch (positionType) { case NO_OPERATION: diff --git a/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/TableChangesHelper.java b/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/TableChangesHelper.java index d38c83d220bf2..80b9c6298dd89 100644 --- a/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/TableChangesHelper.java +++ b/hudi-common/src/main/java/org/apache/hudi/internal/schema/action/TableChangesHelper.java @@ -76,4 +76,9 @@ public static String getParentName(String fullColName) { int offset = fullColName.lastIndexOf("."); return offset > 0 ? fullColName.substring(0, offset) : ""; } + + public static String getLeafName(String fullColName) { + int offset = fullColName.lastIndexOf("."); + return offset > 0 ? fullColName.substring(offset + 1) : fullColName; + } } From c6f08e5fccff1939b1e4589e39c577eeb4905506 Mon Sep 17 00:00:00 2001 From: "wangzixuan.wzxuan" Date: Tue, 13 Sep 2022 00:18:04 +0800 Subject: [PATCH 2/2] add UT --- .../schema/action/TestTableChanges.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/hudi-common/src/test/java/org/apache/hudi/internal/schema/action/TestTableChanges.java b/hudi-common/src/test/java/org/apache/hudi/internal/schema/action/TestTableChanges.java index 5bf817f4d8b28..f8f5a4dc0293a 100644 --- a/hudi-common/src/test/java/org/apache/hudi/internal/schema/action/TestTableChanges.java +++ b/hudi-common/src/test/java/org/apache/hudi/internal/schema/action/TestTableChanges.java @@ -20,8 +20,11 @@ import org.apache.hudi.internal.schema.HoodieSchemaException; import org.apache.hudi.internal.schema.InternalSchema; +import org.apache.hudi.internal.schema.Type; import org.apache.hudi.internal.schema.Types; +import org.apache.hudi.internal.schema.Types.StringType; +import org.apache.hudi.internal.schema.action.TableChange.ColumnPositionChange.ColumnPositionType; import org.apache.hudi.internal.schema.utils.SchemaChangeUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Assertions; @@ -225,5 +228,88 @@ public void testNestUpdate() { ); Assertions.assertEquals(newSchema.getRecord(), checkSchema.getRecord()); } + + @Test + public void testChangeApplier() { + // We add test here to verify the logic of applyAddChange and applyReOrderColPositionChange + InternalSchema oldSchema = new InternalSchema(Types.Field.get(0, false, "id", Types.IntType.get()), + Types.Field.get(1, true, "data", Types.StringType.get()), + Types.Field.get(2, true, "preferences", + Types.RecordType.get(Types.Field.get(7, false, "feature1", + Types.BooleanType.get()), Types.Field.get(8, true, "feature2", Types.BooleanType.get()))), + Types.Field.get(3, false, "locations", Types.MapType.get(9, 10, Types.StringType.get(), + Types.RecordType.get(Types.Field.get(11, false, "lat", Types.FloatType.get()), Types.Field.get(12, false, "long", Types.FloatType.get())), false)), + Types.Field.get(4, true, "points", Types.ArrayType.get(13, true, + Types.RecordType.get(Types.Field.get(14, false, "x", Types.LongType.get()), Types.Field.get(15, false, "y", Types.LongType.get())))), + Types.Field.get(5, false,"doubles", Types.ArrayType.get(16, false, Types.DoubleType.get())), + Types.Field.get(6, true, "properties", Types.MapType.get(17, 18, Types.StringType.get(), Types.StringType.get())) + ); + + // add c1 first + InternalSchema newSchema = addOperationForSchemaChangeApplier(oldSchema, "c1", StringType.get(), "add c1 first", + "id", ColumnPositionType.BEFORE); + //add preferences.cx before preferences.feature2 + newSchema = addOperationForSchemaChangeApplier(newSchema, "preferences.cx", Types.BooleanType.get(), "add preferences.cx before preferences.feature2", + "preferences.feature2", ColumnPositionType.BEFORE); + // check repeated add. + InternalSchema currSchema = newSchema; + Assertions.assertThrows(HoodieSchemaException.class, () -> addOperationForSchemaChangeApplier(currSchema, "preferences.cx", Types.BooleanType.get(), + "add preferences.cx before preferences.feature2")); + // add locations.value.lax before locations.value.long + newSchema = addOperationForSchemaChangeApplier(newSchema, "locations.value.lax", Types.BooleanType.get(), "add locations.value.lax before locations.value.long"); + newSchema = reOrderOperationForSchemaChangeApplier(newSchema, "locations.value.lax", "locations.value.long", ColumnPositionType.BEFORE); + // + // add points.element.z after points.element.y + newSchema = addOperationForSchemaChangeApplier(newSchema, "points.element.z", Types.BooleanType.get(), "add points.element.z after points.element.y", "points.element.y", ColumnPositionType.AFTER); + InternalSchema checkedSchema = new InternalSchema( + Types.Field.get(19, true, "c1", Types.StringType.get(), "add c1 first"), + Types.Field.get(0, false, "id", Types.IntType.get()), + Types.Field.get(1, true, "data", Types.StringType.get()), + Types.Field.get(2, true, "preferences", + Types.RecordType.get(Types.Field.get(7, false, "feature1", Types.BooleanType.get()), + Types.Field.get(20, true, "cx", Types.BooleanType.get(), "add preferences.cx before preferences.feature2"), + Types.Field.get(8, true, "feature2", Types.BooleanType.get()))), + Types.Field.get(3, false, "locations", Types.MapType.get(9, 10, Types.StringType.get(), + Types.RecordType.get(Types.Field.get(11, false, "lat", Types.FloatType.get()), + Types.Field.get(21, true, "lax", Types.BooleanType.get(), "add locations.value.lax before locations.value.long"), + Types.Field.get(12, false, "long", Types.FloatType.get())), false)), + Types.Field.get(4, true, "points", Types.ArrayType.get(13, true, + Types.RecordType.get(Types.Field.get(14, false, "x", Types.LongType.get()), + Types.Field.get(15, false, "y", Types.LongType.get()), + Types.Field.get(22, true, "z", Types.BooleanType.get(), "add points.element.z after points.element.y")))), + Types.Field.get(5, false,"doubles", Types.ArrayType.get(16, false, Types.DoubleType.get())), + Types.Field.get(6, true, "properties", Types.MapType.get(17, 18, Types.StringType.get(), Types.StringType.get())) + ); + Assertions.assertEquals(newSchema.getRecord(), checkedSchema.getRecord()); + } + + private static InternalSchema addOperationForSchemaChangeApplier( + InternalSchema schema, + String colName, + Type colType, + String doc, + String position, + TableChange.ColumnPositionChange.ColumnPositionType positionType) { + InternalSchemaChangeApplier applier = new InternalSchemaChangeApplier(schema); + return applier.applyAddChange(colName, colType, doc, position, positionType); + } + + private static InternalSchema reOrderOperationForSchemaChangeApplier( + InternalSchema schema, + String colName, + String position, + TableChange.ColumnPositionChange.ColumnPositionType positionType) { + InternalSchemaChangeApplier applier = new InternalSchemaChangeApplier(schema); + return applier.applyReOrderColPositionChange(colName, position, positionType); + } + + private static InternalSchema addOperationForSchemaChangeApplier( + InternalSchema schema, + String colName, + Type colType, + String doc) { + return addOperationForSchemaChangeApplier(schema, colName, colType, doc, "", + ColumnPositionType.NO_OPERATION); + } }