This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-2c7c308d-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 6b556fdff1c6008ea66a4b91cf336fe2191379bc Author: Yuhui <[email protected]> AuthorDate: Thu Oct 22 09:50:53 2026 +0800 [#14073] fix(catalog-jdbc): Quote reference column when updating column position (#14075) ### What changes were proposed in this pull request? Quote the reference column of `AFTER` when updating a column position in MySQL, Doris and StarRocks, matching the add-column paths. ### Why are the changes needed? Moving a column after a column named like `order-id` or `key` generates invalid SQL and fails. Fix: #14073 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added docker tests in `TestMysqlTableOperations`, `TestDorisTableOperations` and `TestStarRocksTableOperations`. They fail without this change and pass with it. # Conflicts: # catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java # catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java --- .../doris/operation/DorisTableOperations.java | 8 +- .../doris/operation/TestDorisTableOperations.java | 38 +++++++ .../mysql/operation/MysqlTableOperations.java | 4 +- .../mysql/operation/TestMysqlTableOperations.java | 77 ++++++++++++++ .../operations/StarRocksTableOperations.java | 8 +- .../operation/TestStarRocksTableOperations.java | 112 +++++++++++++++++++++ 6 files changed, 243 insertions(+), 4 deletions(-) diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java index 826c3ea5e8..ac0949beb9 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java @@ -1139,7 +1139,13 @@ public class DorisTableOperations extends JdbcTableOperations { Arrays.stream(jdbcTable.columns()) .reduce((column1, column2) -> column2) .map(Column::name) - .ifPresent(s -> columnDefinition.append("AFTER ").append(s)); + .ifPresent( + s -> + columnDefinition + .append("AFTER ") + .append(BACK_QUOTE) + .append(s) + .append(BACK_QUOTE)); } return columnDefinition.toString(); } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java index 20ddc08631..65a15ef993 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java @@ -41,6 +41,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcTable; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; import org.apache.gravitino.catalog.jdbc.operation.JdbcTablePartitionOperations; import org.apache.gravitino.integration.test.util.GravitinoITUtils; +import org.apache.gravitino.rel.Column; import org.apache.gravitino.rel.TableChange; import org.apache.gravitino.rel.expressions.Expression; import org.apache.gravitino.rel.expressions.NamedReference; @@ -199,6 +200,43 @@ public class TestDorisTableOperations extends TestDoris { TABLE_OPERATIONS.drop(databaseName, newName), "table should be non-existent"); } + @Test + public void testUpdateColumnPositionAfterReservedWordColumn() { + String tableName = GravitinoITUtils.genRandomName("doris_position_test_table"); + // The wide col_2 keeps status and key out of the default key columns. + JdbcColumn[] columns = + new JdbcColumn[] { + JdbcColumn.builder().withName("col_1").withType(INT).build(), + JdbcColumn.builder().withName("col_2").withType(VARCHAR_255).build(), + JdbcColumn.builder().withName("status").withType(VARCHAR_255).build(), + JdbcColumn.builder().withName("key").withType(VARCHAR_255).build() + }; + TABLE_OPERATIONS.create( + databaseName, + tableName, + columns, + "test_comment", + createProperties(), + null, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1")), + new Index[0]); + + // A null position moves the column after the last column `key`, a reserved word. + TABLE_OPERATIONS.alterTable( + databaseName, tableName, TableChange.updateColumnPosition(new String[] {"status"}, null)); + + Awaitility.await() + .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS) + .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS) + .untilAsserted( + () -> + Assertions.assertArrayEquals( + new String[] {"col_1", "col_2", "key", "status"}, + Arrays.stream(TABLE_OPERATIONS.load(databaseName, tableName).columns()) + .map(Column::name) + .toArray(String[]::new))); + } + @Test public void testAlterTable() { String tableName = GravitinoITUtils.genRandomName("doris_alter_test_table"); diff --git a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java index c3e9b7898d..27f2936c6d 100644 --- a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java +++ b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java @@ -654,12 +654,12 @@ public class MysqlTableOperations extends JdbcTableOperations { columnDefinition.append("FIRST"); } else if (updateColumnPosition.getPosition() instanceof TableChange.After) { TableChange.After afterPosition = (TableChange.After) updateColumnPosition.getPosition(); - columnDefinition.append(AFTER).append(afterPosition.getColumn()); + columnDefinition.append(AFTER).append(quote(afterPosition.getColumn())); } else { Arrays.stream(jdbcTable.columns()) .reduce((column1, column2) -> column2) .map(Column::name) - .ifPresent(s -> columnDefinition.append(AFTER).append(s)); + .ifPresent(s -> columnDefinition.append(AFTER).append(quote(s))); } return columnDefinition.toString(); } diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java index 4fe9f4b8bf..78d9a3935b 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java @@ -198,6 +198,83 @@ public class TestMysqlTableOperations extends TestMysql { } @Test +<<<<<<< HEAD +======= + public void testAlterTableUpdateColumnTypeKeepsDefaultValue() { + String tableName = RandomStringUtils.randomAlphabetic(16) + "_type_default_table"; + JdbcColumn col_1 = + JdbcColumn.builder().withName("col_1").withType(INT).withNullable(false).build(); + JdbcColumn col_2 = + JdbcColumn.builder() + .withName("col_2") + .withType(Types.VarCharType.of(16)) + .withDefaultValue(Literals.of("new", Types.VarCharType.of(16))) + .withNullable(false) + .build(); + TABLE_OPERATIONS.create( + TEST_DB_NAME.toString(), + tableName, + new JdbcColumn[] {col_1, col_2}, + "test_comment", + new HashMap<>(), + null, + Distributions.NONE, + new Index[0]); + + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME.toString(), + tableName, + TableChange.updateColumnType(new String[] {col_2.name()}, Types.VarCharType.of(32))); + + JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Column updated = load.columns()[1]; + Assertions.assertEquals(Types.VarCharType.of(32), updated.dataType()); + Assertions.assertEquals(Literals.of("new", Types.VarCharType.of(32)), updated.defaultValue()); + } + + @Test + public void testUpdateColumnPositionWithSpecialReferenceColumn() { + String tableName = RandomStringUtils.randomAlphabetic(16) + "_position_table"; + JdbcColumn[] columns = + new JdbcColumn[] { + JdbcColumn.builder().withName("id").withType(INT).withNullable(false).build(), + JdbcColumn.builder().withName("status").withType(VARCHAR).build(), + JdbcColumn.builder().withName("order-id").withType(VARCHAR).build(), + JdbcColumn.builder().withName("key").withType(VARCHAR).build() + }; + TABLE_OPERATIONS.create( + TEST_DB_NAME.toString(), + tableName, + columns, + "test_comment", + new HashMap<>(), + null, + Distributions.NONE, + new Index[0]); + + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME.toString(), + tableName, + TableChange.updateColumnPosition( + new String[] {"status"}, TableChange.ColumnPosition.after("order-id"))); + JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Assertions.assertArrayEquals( + new String[] {"id", "order-id", "status", "key"}, + Arrays.stream(load.columns()).map(Column::name).toArray(String[]::new)); + + // A null position moves the column after the last column `key`, a reserved word. + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME.toString(), + tableName, + TableChange.updateColumnPosition(new String[] {"id"}, null)); + load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Assertions.assertArrayEquals( + new String[] {"order-id", "status", "key", "id"}, + Arrays.stream(load.columns()).map(Column::name).toArray(String[]::new)); + } + + @Test +>>>>>>> 2c7c308d9 ([#14073] fix(catalog-jdbc): Quote reference column when updating column position (#14075)) public void testAlterTable() { String tableName = RandomStringUtils.randomAlphabetic(16) + "_al_table"; String tableComment = "test_comment"; diff --git a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java index 2d978c94b5..8ba17b30a2 100644 --- a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java +++ b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java @@ -591,7 +591,13 @@ public class StarRocksTableOperations extends JdbcTableOperations { Arrays.stream(jdbcTable.columns()) .reduce((column1, column2) -> column2) .map(Column::name) - .ifPresent(s -> columnDefinition.append("AFTER ").append(s)); + .ifPresent( + s -> + columnDefinition + .append("AFTER ") + .append(BACK_QUOTE) + .append(s) + .append(BACK_QUOTE)); } return columnDefinition.toString(); } diff --git a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java index 934cc425da..da7b8da576 100644 --- a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java +++ b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java @@ -252,6 +252,118 @@ public class TestStarRocksTableOperations extends TestStarRocks { } @Test +<<<<<<< HEAD +======= + public void testClearTableCommentWithNull() { + String tableName = GravitinoITUtils.genRandomName("starrocks_clear_comment_table"); + JdbcColumn column = + JdbcColumn.builder().withName("col_1").withType(INT).withComment("id").build(); + + try { + TABLE_OPERATIONS.create( + databaseName, + tableName, + new JdbcColumn[] {column}, + "table comment", + createProperties(), + null, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1")), + Indexes.EMPTY_INDEXES); + + TABLE_OPERATIONS.alterTable(databaseName, tableName, TableChange.updateComment(null)); + Awaitility.await() + .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS) + .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS) + .untilAsserted( + () -> { + String comment = TABLE_OPERATIONS.load(databaseName, tableName).comment(); + Assertions.assertTrue(StringUtils.isEmpty(comment), comment); + }); + } finally { + TABLE_OPERATIONS.drop(databaseName, tableName); + } + } + + @Test + public void testAlterTableUpdateColumnTypeKeepsDefaultValue() { + String tableName = GravitinoITUtils.genRandomName("starrocks_type_default_test_table"); + JdbcColumn col_1 = JdbcColumn.builder().withName("col_1").withType(INT).build(); + // The wide col_2 keeps col_3 out of the default key columns, so col_3 is a value column. + JdbcColumn col_2 = JdbcColumn.builder().withName("col_2").withType(VARCHAR_255).build(); + JdbcColumn col_3 = + JdbcColumn.builder() + .withName("col_3") + .withType(Types.VarCharType.of(16)) + .withDefaultValue(Literals.of("new", Types.VarCharType.of(16))) + .build(); + TABLE_OPERATIONS.create( + databaseName, + tableName, + new JdbcColumn[] {col_1, col_2, col_3}, + "test_comment", + createProperties(), + null, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1")), + new Index[0]); + + TABLE_OPERATIONS.alterTable( + databaseName, + tableName, + TableChange.updateColumnType(new String[] {col_3.name()}, Types.VarCharType.of(32))); + + // StarRocks applies schema changes asynchronously. + Awaitility.await() + .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS) + .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS) + .untilAsserted( + () -> { + Column updated = TABLE_OPERATIONS.load(databaseName, tableName).columns()[2]; + Assertions.assertEquals(Types.VarCharType.of(32), updated.dataType()); + Assertions.assertEquals( + Literals.of("new", Types.VarCharType.of(32)), updated.defaultValue()); + }); + } + + @Test + public void testUpdateColumnPositionAfterReservedWordColumn() { + String tableName = GravitinoITUtils.genRandomName("starrocks_position_test_table"); + // The wide col_2 keeps status and key out of the default key columns. + JdbcColumn[] columns = + new JdbcColumn[] { + JdbcColumn.builder().withName("col_1").withType(INT).build(), + JdbcColumn.builder().withName("col_2").withType(VARCHAR_255).build(), + JdbcColumn.builder().withName("status").withType(VARCHAR_255).build(), + JdbcColumn.builder().withName("key").withType(VARCHAR_255).build() + }; + TABLE_OPERATIONS.create( + databaseName, + tableName, + columns, + "test_comment", + createProperties(), + null, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1")), + new Index[0]); + + // A null position moves the column after the last column `key`, a reserved word. + TABLE_OPERATIONS.alterTable( + databaseName, tableName, TableChange.updateColumnPosition(new String[] {"status"}, null)); + + // StarRocks applies schema changes asynchronously. + Awaitility.await() + .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS) + .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS) + .untilAsserted( + () -> + Assertions.assertArrayEquals( + new String[] {"col_1", "col_2", "key", "status"}, + Arrays.stream(TABLE_OPERATIONS.load(databaseName, tableName).columns()) + .map(Column::name) + .toArray(String[]::new))); + } + + @Test +>>>>>>> 2c7c308d9 ([#14073] fix(catalog-jdbc): Quote reference column when updating column position (#14075)) public void testAlterTable() { String tableName = GravitinoITUtils.genRandomName("starrocks_alter_test_table");
