This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 6a9931414e [Cherry-pick to branch-1.3] [#14062][#14063]
fix(starrocks): Quote rename column names and skip empty alter clauses (#14064)
(#14134)
6a9931414e is described below
commit 6a9931414e6e038102fa9c35965049320e786d42
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Oct 23 22:54:49 2026 +0800
[Cherry-pick to branch-1.3] [#14062][#14063] fix(starrocks): Quote rename
column names and skip empty alter clauses (#14064) (#14134)
**Cherry-pick Information:**
- Original commit: 3a445d8899218929ab0bceb48d8d0bd708918fff
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Yuhui <[email protected]>
---
.../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 8d7b39ca2e..c241b41904 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 d2665ca33c..68afce08bb 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();
@@ -375,4 +393,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);
+ }
}
