This is an automated email from the ASF dual-hosted git repository.
diqiu50 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 2c7c308d9c [#14073] fix(catalog-jdbc): Quote reference column when
updating column position (#14075)
2c7c308d9c is described below
commit 2c7c308d9c56866e8df2f0ebf4566a7c917cd29b
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.
---
.../doris/operation/DorisTableOperations.java | 8 ++++-
.../doris/operation/TestDorisTableOperations.java | 38 ++++++++++++++++++++
.../mysql/operation/MysqlTableOperations.java | 4 +--
.../mysql/operation/TestMysqlTableOperations.java | 41 ++++++++++++++++++++++
.../operations/StarRocksTableOperations.java | 8 ++++-
.../operation/TestStarRocksTableOperations.java | 38 ++++++++++++++++++++
6 files changed, 133 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 da91101b37..380807a6e6 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
@@ -1188,7 +1188,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 b2d4081525..f5fcbaff39 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
@@ -42,6 +42,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;
@@ -200,6 +201,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 41d7746fa6..73fe9d3d12 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
@@ -728,12 +728,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 c24e2b0365..d76eedc8c4 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
@@ -232,6 +232,47 @@ public class TestMysqlTableOperations extends TestMysql {
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
public void testAlterTable() {
String tableName = RandomStringUtils.randomAlphabetic(16) + "_al_table";
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 f1c628a4b1..71551d036f 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
@@ -600,7 +600,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 60f5338f54..fb4a4c0ae4 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
@@ -324,6 +324,44 @@ public class TestStarRocksTableOperations extends
TestStarRocks {
});
}
+ @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
public void testAlterTable() {
String tableName =
GravitinoITUtils.genRandomName("starrocks_alter_test_table");
