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 c3843d68ee [#13999] fix(starrocks): Apply nullability and default
value when adding columns (#14004)
c3843d68ee is described below
commit c3843d68eedca922e35f02df3e6a9b3b4c37d22b
Author: Yuhui <[email protected]>
AuthorDate: Wed Oct 21 09:23:24 2026 +0800
[#13999] fix(starrocks): Apply nullability and default value when adding
columns (#14004)
### What changes were proposed in this pull request?
Build the StarRocks `ADD COLUMN` clause with `appendColumnDefinition`,
the same logic used by
create table, so nullability and default value are applied. Reject
auto-increment columns
explicitly since StarRocks does not support adding them.
### Why are the changes needed?
StarRocks add column silently dropped nullability, default value and
auto-increment.
Fix: #13999
### Does this PR introduce _any_ user-facing change?
Yes. Added StarRocks columns now honor nullability and default value.
Adding an auto-increment
column now fails with a clear error.
### How was this patch tested?
- Added unit tests in `TestStarRocksTableOperationsSqlGeneration`.
- Extended `CatalogStarRocksIT#testColumnDefaultValue`.
---
.../operations/StarRocksTableOperations.java | 35 ++++++-----
.../integration/test/CatalogStarRocksIT.java | 28 +++++++++
.../TestStarRocksTableOperationsSqlGeneration.java | 67 ++++++++++++++++++++++
3 files changed, 112 insertions(+), 18 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 439deee4dd..c1a9e06bf2 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
@@ -474,29 +474,28 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
}
private String addColumnFieldDefinition(TableChange.AddColumn addColumn) {
- String dataType = typeConverter.fromGravitino(addColumn.getDataType());
if (addColumn.fieldName().length > 1) {
throw new UnsupportedOperationException("StarRocks does not support
nested column names.");
}
String col = addColumn.fieldName()[0];
+ // StarRocks only allows AUTO_INCREMENT columns to be defined at table
creation time
+ Preconditions.checkArgument(
+ !addColumn.isAutoIncrement(),
+ "StarRocks does not support adding an auto-increment column: %s",
+ col);
- StringBuilder columnDefinition = new StringBuilder();
- columnDefinition
- .append("ADD COLUMN ")
- .append(BACK_QUOTE)
- .append(col)
- .append(BACK_QUOTE)
- .append(SPACE)
- .append(dataType)
- .append(SPACE);
-
- // Append comment if available
- if (StringUtils.isNotEmpty(addColumn.getComment())) {
- columnDefinition
- .append("COMMENT '")
- .append(escapeSqlLiteral(addColumn.getComment(), '\''))
- .append("' ");
- }
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName(col)
+ .withType(addColumn.getDataType())
+ .withComment(addColumn.getComment())
+ .withNullable(addColumn.isNullable())
+ .withDefaultValue(addColumn.getDefaultValue())
+ .withAutoIncrement(false)
+ .build();
+ StringBuilder columnDefinition =
+ new StringBuilder("ADD COLUMN
").append(BACK_QUOTE).append(col).append(BACK_QUOTE);
+ appendColumnDefinition(column, columnDefinition);
// Append position if available
if (addColumn.getPosition() instanceof TableChange.First) {
diff --git
a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/integration/test/CatalogStarRocksIT.java
b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/integration/test/CatalogStarRocksIT.java
index 28369a00c8..81c0c8dffb 100644
---
a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/integration/test/CatalogStarRocksIT.java
+++
b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/integration/test/CatalogStarRocksIT.java
@@ -1004,5 +1004,33 @@ public class CatalogStarRocksIT extends BaseIT {
Literals.timestampLiteral(LocalDateTime.of(2024, 1, 1, 1, 1, 1)),
colDefaultValues[1].defaultValue());
Assertions.assertEquals(DEFAULT_VALUE_OF_CURRENT_TIMESTAMP,
colDefaultValues[2].defaultValue());
+
+ // add a column with nullability and default value
+ tableCatalog.alterTable(
+ tableIdentifier,
+ TableChange.addColumn(
+ new String[] {"col_add_default_value"},
+ Types.TimestampType.withoutTimeZone(),
+ "col_add_default_value",
+ TableChange.ColumnPosition.defaultPos(),
+ false,
+ false,
+ Literals.timestampLiteral("2024-01-01T01:01:01")));
+
+ Awaitility.await()
+ .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS)
+ .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
+ .untilAsserted(
+ () -> {
+ Column addedColumn =
+
Arrays.stream(tableCatalog.loadTable(tableIdentifier).columns())
+ .filter(c -> c.name().equals("col_add_default_value"))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Column is not
added"));
+ Assertions.assertFalse(addedColumn.nullable());
+ Assertions.assertEquals(
+ Literals.timestampLiteral(LocalDateTime.of(2024, 1, 1, 1, 1,
1)),
+ addedColumn.defaultValue());
+ });
}
}
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 fa2d6c40f8..cebe5e2fa3 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
@@ -29,6 +29,7 @@ import
org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.jdbc.utils.ConnectionCountingDataSource;
import org.apache.gravitino.catalog.starrocks.converter.StarRocksTypeConverter;
import
org.apache.gravitino.catalog.starrocks.operations.StarRocksTableOperations;
+import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.NamedReference;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
@@ -293,4 +294,70 @@ public class TestStarRocksTableOperationsSqlGeneration {
Assertions.assertTrue(alterSql.contains("ADD COLUMN `col2`"), alterSql);
Assertions.assertTrue(alterSql.contains("COMMENT 'owner\\\\''s
\"comment\"; --'"), alterSql);
}
+
+ @Test
+ public void testAddColumnWithNotNullAndDefaultValue() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(
+ new String[] {"created_at"},
+ Types.TimestampType.withoutTimeZone(),
+ "create time",
+ TableChange.ColumnPosition.after("id"),
+ false,
+ false,
+ Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP));
+
+ Assertions.assertTrue(
+ alterSql.contains(
+ "ADD COLUMN `created_at` datetime NOT NULL DEFAULT
CURRENT_TIMESTAMP "
+ + "COMMENT 'create time' AFTER `id`"),
+ alterSql);
+ }
+
+ @Test
+ public void testAddNullableColumnWithStringDefaultValue() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(
+ new String[] {"col2"},
+ Types.VarCharType.of(10),
+ null,
+ TableChange.ColumnPosition.after("col1"),
+ true,
+ false,
+ Literals.of("abc", Types.VarCharType.of(10))));
+
+ Assertions.assertTrue(
+ alterSql.contains("ADD COLUMN `col2` varchar(10) NULL DEFAULT 'abc'
AFTER `col1`"),
+ alterSql);
+ }
+
+ @Test
+ public void testAddAutoIncrementColumnIsRejected() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(
+ new String[] {"col2"},
+ Types.LongType.get(),
+ null,
+ TableChange.ColumnPosition.defaultPos(),
+ false,
+ true)));
+
+ Assertions.assertEquals(
+ "StarRocks does not support adding an auto-increment column: col2",
exception.getMessage());
+ }
}
