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 3a445d8899 [#14062][#14063] fix(starrocks): Quote rename column names
and skip empty alter clauses (#14064)
3a445d8899 is described below
commit 3a445d8899218929ab0bceb48d8d0bd708918fff
Author: Yuhui <[email protected]>
AuthorDate: Thu Oct 22 09:50:30 2026 +0800
[#14062][#14063] fix(starrocks): Quote rename column names and skip empty
alter clauses (#14064)
### What changes were proposed in this pull request?
- Quote both column names in StarRocks rename column SQL.
- Skip empty delete column clauses, and return an empty SQL when no
change remains so no ALTER TABLE is sent.
### Why are the changes needed?
Unquoted names break renames of special or reserved column names, and an
empty `ALTER TABLE` is executed by StarRocks as a table OPTIMIZE.
Fix: #14062, #14063
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added unit tests in `TestStarRocksTableOperationsSqlGeneration`.
---
.../operations/StarRocksTableOperations.java | 14 ++++-
.../TestStarRocksTableOperationsSqlGeneration.java | 59 +++++++++++++++++++++-
2 files changed, 69 insertions(+), 4 deletions(-)
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 5df1941410..f1c628a4b1 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
@@ -153,7 +153,10 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
} else if (change instanceof TableChange.DeleteColumn) {
TableChange.DeleteColumn deleteColumn = (TableChange.DeleteColumn)
change;
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
- alterSql.add(deleteColumnFieldDefinition(deleteColumn, lazyLoadTable));
+ String deleteColSql = deleteColumnFieldDefinition(deleteColumn,
lazyLoadTable);
+ if (StringUtils.isNotEmpty(deleteColSql)) {
+ alterSql.add(deleteColSql);
+ }
} else if (change instanceof TableChange.RemoveProperty) {
throw new IllegalArgumentException("Remove property is not supported
yet.");
} else if (change instanceof TableChange.RenameColumn) {
@@ -210,6 +213,10 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
}
}
+ // An empty ALTER TABLE statement is treated as OPTIMIZE by StarRocks, so
skip it.
+ if (alterSql.isEmpty()) {
+ return "";
+ }
String result = "ALTER TABLE `" + tableName + "`\n" + String.join(",\n",
alterSql) + ";";
LOG.info("Generated alter table:{}.{} sql: {}", databaseName, tableName,
result);
return result;
@@ -546,7 +553,10 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
try {
getJdbcColumnFromTable(jdbcTable, renameColumn.getNewName());
} catch (NoSuchColumnException ex) {
- return String.format("RENAME COLUMN %s TO %s", oldColName,
renameColumn.getNewName());
+ return "RENAME COLUMN "
+ + quoteIdentifier(oldColName)
+ + " TO "
+ + quoteIdentifier(renameColumn.getNewName());
}
throw new IllegalArgumentException("Column already exists: " +
renameColumn.getNewName());
}
diff --git
a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java
index 0679214d40..36ab0c3b84 100644
---
a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.catalog.starrocks.operation;
import java.sql.Connection;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.apache.gravitino.StringIdentifier;
@@ -50,14 +51,22 @@ public class TestStarRocksTableOperationsSqlGeneration {
private static class TestableStarRocksTableOperations extends
StarRocksTableOperations {
+ private final String[] columnNames;
+
public TestableStarRocksTableOperations() {
+ this(new String[0]);
+ }
+
+ private TestableStarRocksTableOperations(String... columnNames) {
super.exceptionMapper = new JdbcExceptionConverter();
super.typeConverter = new StarRocksTypeConverter();
super.columnDefaultValueConverter = new
JdbcColumnDefaultValueConverter();
+ this.columnNames = columnNames;
}
- private TestableStarRocksTableOperations(ConnectionCountingDataSource
connections) {
- this();
+ private TestableStarRocksTableOperations(
+ ConnectionCountingDataSource connections, String... columnNames) {
+ this(columnNames);
dataSource = connections.dataSource();
}
@@ -96,6 +105,15 @@ public class TestStarRocksTableOperationsSqlGeneration {
String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
return JdbcTable.builder()
.withName(tableName)
+ .withColumns(
+ Arrays.stream(columnNames)
+ .map(
+ name ->
+ JdbcColumn.builder()
+ .withName(name)
+ .withType(Types.IntegerType.get())
+ .build())
+ .toArray(JdbcColumn[]::new))
.withComment(
StringIdentifier.addToComment(StringIdentifier.fromId(42),
"existing comment"))
.build();
@@ -391,4 +409,41 @@ public class TestStarRocksTableOperationsSqlGeneration {
Assertions.assertEquals(
"StarRocks does not support adding an auto-increment column: col2",
exception.getMessage());
}
+
+ @Test
+ void testRenameColumnQuotesIdentifiers() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations("id", "order-id");
+
+ String alterSql =
+ ops.alterTableSql("test_table", TableChange.renameColumn(new String[]
{"order-id"}, "key"));
+
+ Assertions.assertEquals(
+ "ALTER TABLE `test_table`\nRENAME COLUMN `order-id` TO `key`;",
alterSql);
+ }
+
+ @Test
+ void testDeleteMissingColumnIfExistsSkipsAlter() {
+ ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
+ TestableStarRocksTableOperations ops =
+ new TestableStarRocksTableOperations(connections, "id", "col1");
+ TableChange deleteMissing = TableChange.deleteColumn(new String[]
{"missing"}, true);
+
+ Assertions.assertEquals("", ops.alterTableSql("test_table",
deleteMissing));
+
+ ops.alterTable("database", "test_table", deleteMissing);
+ Assertions.assertTrue(connections.executedSql().isEmpty());
+ }
+
+ @Test
+ void testDeleteMissingColumnIfExistsWithOtherChanges() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations("id", "col1");
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.deleteColumn(new String[] {"missing"}, true),
+ TableChange.deleteColumn(new String[] {"col1"}, false));
+
+ Assertions.assertEquals("ALTER TABLE `test_table`\nDROP COLUMN `col1`;",
alterSql);
+ }
}
