LuciferYang commented on code in PR #13448:
URL: https://github.com/apache/gravitino/pull/13448#discussion_r4082610412
##########
core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java:
##########
@@ -336,6 +336,52 @@ private Column createColumn(String name, Type dataType,
Expression defaultValue)
.build();
}
+ @Test
+ public void testAlterTableBundledColumnChanges() {
+ NameIdentifier tableIdent =
+ NameIdentifierUtil.ofTable(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME,
"bundled");
+ Column[] columns =
+ new Column[] {
+ createColumn("col1", Types.StringType.get(), null),
+ createColumn("col2", Types.IntegerType.get(),
Literals.integerLiteral(1)),
+ createColumn("col3", Types.StringType.get(), null)
+ };
+ tableOperations.createTable(
+ tableIdent,
+ columns,
+ "Test bundled alters",
+ StringIdentifier.newPropertiesWithId(
+ StringIdentifier.fromId(idGenerator.nextId()),
Collections.emptyMap()),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0],
+ Indexes.EMPTY_INDEXES);
+
+ // A delete of a lower-position column followed by an update of a
+ // higher-position one must neither crash nor reorder the remaining
columns.
+ Table updated =
+ tableOperations.alterTable(
+ tableIdent,
+ TableChange.deleteColumn(new String[] {"col1"}, false),
+ TableChange.updateColumnComment(new String[] {"col3"}, "updated"));
+ Assertions.assertArrayEquals(
+ new String[] {"col2", "col3"},
+ Arrays.stream(updated.columns()).map(Column::name).toArray());
+
+ // An add at first() followed by an update of a later column must keep
order.
+ Table updated2 =
+ tableOperations.alterTable(
+ tableIdent,
+ TableChange.addColumn(
+ new String[] {"colNew"},
+ Types.StringType.get(),
+ TableChange.ColumnPosition.first()),
+ TableChange.updateColumnComment(new String[] {"col3"}, "updated
again"));
Review Comment:
Done, see the added comment assertions (col3 is `"updated"` then `"updated
again"`).
##########
core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java:
##########
@@ -336,6 +336,52 @@ private Column createColumn(String name, Type dataType,
Expression defaultValue)
.build();
}
+ @Test
+ public void testAlterTableBundledColumnChanges() {
+ NameIdentifier tableIdent =
+ NameIdentifierUtil.ofTable(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME,
"bundled");
+ Column[] columns =
+ new Column[] {
+ createColumn("col1", Types.StringType.get(), null),
+ createColumn("col2", Types.IntegerType.get(),
Literals.integerLiteral(1)),
+ createColumn("col3", Types.StringType.get(), null)
+ };
+ tableOperations.createTable(
+ tableIdent,
+ columns,
+ "Test bundled alters",
+ StringIdentifier.newPropertiesWithId(
+ StringIdentifier.fromId(idGenerator.nextId()),
Collections.emptyMap()),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0],
+ Indexes.EMPTY_INDEXES);
+
+ // A delete of a lower-position column followed by an update of a
+ // higher-position one must neither crash nor reorder the remaining
columns.
+ Table updated =
+ tableOperations.alterTable(
+ tableIdent,
+ TableChange.deleteColumn(new String[] {"col1"}, false),
+ TableChange.updateColumnComment(new String[] {"col3"}, "updated"));
+ Assertions.assertArrayEquals(
+ new String[] {"col2", "col3"},
+ Arrays.stream(updated.columns()).map(Column::name).toArray());
Review Comment:
Done. Both assertions now collect with `toArray(String[]::new)` so the array
type matches the expected `String[]`.
##########
core/src/test/java/org/apache/gravitino/catalog/TestManagedTableOperations.java:
##########
@@ -336,6 +336,52 @@ private Column createColumn(String name, Type dataType,
Expression defaultValue)
.build();
}
+ @Test
+ public void testAlterTableBundledColumnChanges() {
+ NameIdentifier tableIdent =
+ NameIdentifierUtil.ofTable(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME,
"bundled");
+ Column[] columns =
+ new Column[] {
+ createColumn("col1", Types.StringType.get(), null),
+ createColumn("col2", Types.IntegerType.get(),
Literals.integerLiteral(1)),
+ createColumn("col3", Types.StringType.get(), null)
+ };
+ tableOperations.createTable(
+ tableIdent,
+ columns,
+ "Test bundled alters",
+ StringIdentifier.newPropertiesWithId(
+ StringIdentifier.fromId(idGenerator.nextId()),
Collections.emptyMap()),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0],
+ Indexes.EMPTY_INDEXES);
+
+ // A delete of a lower-position column followed by an update of a
+ // higher-position one must neither crash nor reorder the remaining
columns.
+ Table updated =
+ tableOperations.alterTable(
+ tableIdent,
+ TableChange.deleteColumn(new String[] {"col1"}, false),
+ TableChange.updateColumnComment(new String[] {"col3"}, "updated"));
+ Assertions.assertArrayEquals(
+ new String[] {"col2", "col3"},
+ Arrays.stream(updated.columns()).map(Column::name).toArray());
+
+ // An add at first() followed by an update of a later column must keep
order.
+ Table updated2 =
+ tableOperations.alterTable(
+ tableIdent,
+ TableChange.addColumn(
+ new String[] {"colNew"},
+ Types.StringType.get(),
+ TableChange.ColumnPosition.first()),
+ TableChange.updateColumnComment(new String[] {"col3"}, "updated
again"));
+ Assertions.assertArrayEquals(
+ new String[] {"colNew", "col2", "col3"},
+ Arrays.stream(updated2.columns()).map(Column::name).toArray());
Review Comment:
Done, same `toArray(String[]::new)` change applied here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
