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 b4097f31b2 [#14048] fix(catalog-jdbc): Clear comment when updating
with null (#14053)
b4097f31b2 is described below
commit b4097f31b2c6b30d30462ee8334c49030aff580d
Author: Yuhui <[email protected]>
AuthorDate: Thu Oct 22 09:49:57 2026 +0800
[#14048] fix(catalog-jdbc): Clear comment when updating with null (#14053)
### What changes were proposed in this pull request?
Treat a null comment in `UpdateComment` / `UpdateColumnComment` as an
empty comment in JDBC catalogs, so it clears the comment. ClickHouse
uses `COMMENT COLUMN` to clear a column comment, since `MODIFY COLUMN`
without a `COMMENT` clause keeps the existing one.
### Why are the changes needed?
A null comment wrote the literal text "null" on PostgreSQL, Doris,
MySQL, StarRocks and OceanBase, and threw NullPointerException on
Hologres and ClickHouse.
Fix: #14048
### Does this PR introduce _any_ user-facing change?
Yes. Updating a table or column comment with null now clears the
comment.
### How was this patch tested?
Added SQL generation unit tests for each dialect and Docker tests that
verify the comment is cleared on real databases.
---
.../operations/ClickHouseTableOperations.java | 8 ++++-
.../operations/TestClickHouseTableOperations.java | 37 ++++++++++++++++++++
.../TestClickHouseTableOperationsUnit.java | 37 ++++++++++++++++++++
.../operation/HologresTableOperations.java | 4 +--
.../operation/TestHologresTableOperations.java | 12 +++++++
.../operation/OceanBaseTableOperations.java | 2 +-
.../operation/TestOceanBaseTableOperations.java | 36 +++++++++++++++++++
.../TestOceanBaseTableOperationsSqlGeneration.java | 9 +++++
.../doris/operation/DorisTableOperations.java | 4 +--
.../doris/operation/TestDorisTableOperations.java | 40 ++++++++++++++++++++++
.../TestDorisTableOperationsSqlGeneration.java | 13 +++++++
.../mysql/operation/MysqlTableOperations.java | 2 +-
.../mysql/operation/TestMysqlTableOperations.java | 31 +++++++++++++++++
.../TestMysqlTableOperationsSqlGeneration.java | 9 +++++
.../operation/PostgreSqlTableOperations.java | 4 +--
.../operation/TestPostgreSqlTableOperations.java | 31 +++++++++++++++++
...TestPostgreSqlTableOperationsSqlGeneration.java | 13 +++++++
.../operations/StarRocksTableOperations.java | 2 +-
.../operation/TestStarRocksTableOperations.java | 32 +++++++++++++++++
.../TestStarRocksTableOperationsSqlGeneration.java | 17 +++++++++
20 files changed, 333 insertions(+), 10 deletions(-)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
index 5570da8825..23272b53b6 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
@@ -1268,7 +1268,7 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
// Last modified comment
if (null != updateComment) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
// Load the existing table once. We need it for two purposes:
// 1. Preserve the Gravitino StringIdentifier embedded in the old
comment, so Gravitino can
// still identify the table after the comment is changed.
@@ -1599,6 +1599,12 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
String col = updateColumnComment.fieldName()[0];
JdbcColumn column = getJdbcColumnFromTable(jdbcTable, col);
+ // appendColumnDefinition omits an empty COMMENT, and MODIFY COLUMN
without COMMENT keeps the
+ // existing one, so use COMMENT COLUMN to clear it.
+ if (StringUtils.isEmpty(newComment)) {
+ return "COMMENT COLUMN %s ''".formatted(quoteIdentifier(col));
+ }
+
JdbcColumn updateColumn =
JdbcColumn.builder()
.withName(col)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
index 8ca2c98ece..031ca47d69 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
@@ -721,6 +721,43 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Assertions.assertTrue(TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(),
tableName));
}
+ @Test
+ public void testClearCommentsWithNull() {
+ String tableName = RandomStringUtils.randomAlphabetic(16) +
"_clear_comment";
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(INT)
+ .withNullable(false)
+ .withComment("col comment")
+ .build();
+ try {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ new JdbcColumn[] {column},
+ "table comment",
+ new HashMap<>(),
+ null,
+ Distributions.NONE,
+ new Index[0],
+ getSortOrders("col_1"));
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ tableName,
+ TableChange.updateColumnComment(new String[] {"col_1"}, null),
+ TableChange.updateComment(null));
+
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(),
tableName);
+ Assertions.assertTrue(StringUtils.isEmpty(load.comment()),
load.comment());
+ Assertions.assertTrue(
+ StringUtils.isEmpty(load.columns()[0].comment()),
load.columns()[0].comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), tableName);
+ }
+ }
+
@Test
public void testCreateAllTypeTable() {
String tableName = RandomNameUtils.genRandomName("type_table_");
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
index b16c407641..f06dd53bd3 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
@@ -2684,6 +2684,43 @@ public class TestClickHouseTableOperationsUnit {
TableChange.updateComment("new comment")));
}
+ @Test
+ void testClearCommentsWithNull() {
+ ExposedClickHouseTableOperations ops = newAlterOps(new HashMap<>());
+
+ String tableSql =
ops.callGenerateAlterTableSql(TableChange.updateComment(null));
+ Assertions.assertTrue(tableSql.contains("MODIFY COMMENT ''"), tableSql);
+
+ String columnSql =
+ ops.callGenerateAlterTableSql(TableChange.updateColumnComment(new
String[] {"id"}, null));
+ Assertions.assertTrue(columnSql.contains("COMMENT COLUMN `id` ''"),
columnSql);
+ }
+
+ @Test
+ void testClearCommentsWithNullOnCluster() {
+ ExposedClickHouseTableOperations ops =
+ newAlterOps(
+ Map.of(
+ ClusterConstants.ON_CLUSTER,
+ "true",
+ ClusterConstants.CLUSTER_NAME,
+ "test_cluster"));
+
+ String tableSql =
ops.callGenerateAlterTableSql(TableChange.updateComment(null));
+ Assertions.assertTrue(
+ tableSql.startsWith("ALTER TABLE `test_table` ON CLUSTER
`test_cluster`"), tableSql);
+ String clusterComment =
+ ClickHouseClusterUtils.escapeSingleQuotes(
+ ClickHouseClusterUtils.embedClusterInComment("", "test_cluster"));
+ Assertions.assertTrue(tableSql.contains("MODIFY COMMENT '" +
clusterComment + "'"), tableSql);
+
+ String columnSql =
+ ops.callGenerateAlterTableSql(TableChange.updateColumnComment(new
String[] {"id"}, null));
+ Assertions.assertTrue(
+ columnSql.startsWith("ALTER TABLE `test_table` ON CLUSTER
`test_cluster`"), columnSql);
+ Assertions.assertTrue(columnSql.contains("COMMENT COLUMN `id` ''"),
columnSql);
+ }
+
@Test
void testRejectDuplicateTableSettingChanges() {
ExposedClickHouseTableOperations ops = newOps();
diff --git
a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/operation/HologresTableOperations.java
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/operation/HologresTableOperations.java
index 1ace95f3b1..4b356a7c0b 100644
---
a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/operation/HologresTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/operation/HologresTableOperations.java
@@ -505,7 +505,7 @@ public class HologresTableOperations extends
JdbcTableOperations
private String updateCommentDefinition(
TableChange.UpdateComment updateComment, JdbcTable jdbcTable) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (null == StringIdentifier.fromComment(newComment)) {
// Detect and add Gravitino id.
if (StringUtils.isNotEmpty(jdbcTable.comment())) {
@@ -608,7 +608,7 @@ public class HologresTableOperations extends
JdbcTableOperations
private String updateColumnCommentFieldDefinition(
TableChange.UpdateColumnComment updateColumnComment, String tableName) {
- String newComment = updateColumnComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateColumnComment.getNewComment());
if (updateColumnComment.fieldName().length > 1) {
throw new
UnsupportedOperationException(HOLOGRES_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
diff --git
a/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/operation/TestHologresTableOperations.java
b/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/operation/TestHologresTableOperations.java
index 99420710ee..79019135f1 100644
---
a/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/operation/TestHologresTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/operation/TestHologresTableOperations.java
@@ -902,6 +902,18 @@ public class TestHologresTableOperations {
assertTrue(sql.contains("IS 'it''s a test'"));
}
+ @Test
+ void testAlterTableClearCommentsWithNull() {
+ String columnSql =
+ ops.alterTableSql(
+ "public", "test_table", TableChange.updateColumnComment(new
String[] {"col1"}, null));
+ assertEquals("COMMENT ON COLUMN \"test_table\".\"col1\" IS '';",
columnSql);
+
+ ops.setMockTable(ops.buildFakeTable("test_table"));
+ String tableSql = ops.alterTableSql("public", "test_table",
TableChange.updateComment(null));
+ assertEquals("COMMENT ON TABLE \"test_table\" IS '';", tableSql);
+ }
+
@Test
void testAlterTableRenameColumnRejectsNestedField() {
assertThrows(
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
index b51544bfee..d1a2e2b523 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
@@ -305,7 +305,7 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
// Last modified comment
if (null != updateComment) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (null == StringIdentifier.fromComment(newComment)) {
// Detect and add Gravitino id.
JdbcTable jdbcTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
index 678f8c4fdb..3a07734d6c 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
@@ -110,6 +110,42 @@ public class TestOceanBaseTableOperations extends
TestOceanBase {
}
}
+ @Test
+ void testClearCommentsWithNull() {
+ String tableName = RandomNameUtils.genRandomName("clear_comment");
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(VARCHAR)
+ .withNullable(true)
+ .withComment("col comment")
+ .build();
+ try {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ tableName,
+ new JdbcColumn[] {column},
+ "table comment",
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ tableName,
+ TableChange.updateColumnComment(new String[] {"col1"}, null),
+ TableChange.updateComment(null));
+
+ JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertTrue(StringUtils.isEmpty(table.comment()),
table.comment());
+ Assertions.assertTrue(
+ StringUtils.isEmpty(table.columns()[0].comment()),
table.columns()[0].comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, tableName);
+ }
+ }
+
@Test
public void testOperationTable() {
String tableName = RandomStringUtils.randomAlphabetic(16).toLowerCase() +
"_op_table";
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
index 612c034022..15cc078b94 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
@@ -93,6 +93,15 @@ public class TestOceanBaseTableOperationsSqlGeneration {
Assertions.assertEquals("ALTER TABLE `test_table`\nCOMMENT '" + escaped +
"';", sql);
}
+ @Test
+ void testClearTableCommentWithNull() {
+ TestableOceanBaseTableOperations ops = operationsWithTable();
+ String sql =
+ ops.generateAlterTableSql("database", "test_table",
TableChange.updateComment(null));
+
+ Assertions.assertEquals("ALTER TABLE `test_table`\nCOMMENT '';", sql);
+ }
+
@ParameterizedTest
@MethodSource("comments")
void testAddColumnEscapesComment(String comment, String escaped) {
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 8e50d615b9..da91101b37 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
@@ -942,7 +942,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
// Last modified comment
if (null != updateComment) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (null == StringIdentifier.fromComment(newComment)) {
// Detect and add Gravitino id.
JdbcTable jdbcTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
@@ -1039,7 +1039,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
private String updateColumnCommentFieldDefinition(
TableChange.UpdateColumnComment updateColumnComment) {
- String newComment = updateColumnComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateColumnComment.getNewComment());
if (updateColumnComment.fieldName().length > 1) {
throw new IllegalArgumentException("Doris does not support nested column
names.");
}
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 36c550882d..b2d4081525 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
@@ -35,6 +35,7 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.catalog.doris.converter.DorisTypeConverter;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
@@ -418,6 +419,45 @@ public class TestDorisTableOperations extends TestDoris {
TABLE_OPERATIONS.load(databaseName, tableName)));
}
+ @Test
+ public void testClearCommentsWithNull() {
+ String tableName =
GravitinoITUtils.genRandomName("doris_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);
+
+ // Doris rejects table and column comment changes in the same ALTER
statement.
+ TABLE_OPERATIONS.alterTable(
+ databaseName, tableName, TableChange.updateColumnComment(new
String[] {"col_1"}, null));
+ 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(
+ () -> {
+ JdbcTable load = TABLE_OPERATIONS.load(databaseName,
tableName);
+ // Doris reports the engine name (OLAP) as the comment of a
table without a comment.
+ Assertions.assertTrue(
+ StringUtils.isEmpty(load.comment()) ||
"OLAP".equals(load.comment()),
+ load.comment());
+ Assertions.assertTrue(
+ StringUtils.isEmpty(load.columns()[0].comment()),
load.columns()[0].comment());
+ });
+ } finally {
+ TABLE_OPERATIONS.drop(databaseName, tableName);
+ }
+ }
+
@Test
public void testCreateAllTypeTable() {
String tableName = GravitinoITUtils.genRandomName("all_type_table");
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
index eeee1d42be..1f5984a772 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
@@ -562,6 +562,19 @@ public class TestDorisTableOperationsSqlGeneration {
alterSql);
}
+ @Test
+ public void testClearCommentsWithNull() {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+
+ String columnSql =
+ ops.alterTableSql(
+ "test_table", TableChange.updateColumnComment(new String[]
{"col1"}, null));
+ Assertions.assertTrue(columnSql.contains("MODIFY COLUMN `col1` COMMENT
''"), columnSql);
+
+ String tableSql = ops.alterTableSql("test_table",
TableChange.updateComment(null));
+ Assertions.assertTrue(tableSql.contains("MODIFY COMMENT \"\""), tableSql);
+ }
+
@Test
public void testEscapeAddColumnCommentInGeneratedSql() {
TestableDorisTableOperations ops = new TestableDorisTableOperations();
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 f54990874c..41d7746fa6 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
@@ -441,7 +441,7 @@ public class MysqlTableOperations extends
JdbcTableOperations {
// Last modified comment
if (null != updateComment) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (null == StringIdentifier.fromComment(newComment)) {
// Detect and add Gravitino id.
JdbcTable jdbcTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
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 580886719d..c24e2b0365 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
@@ -678,6 +678,37 @@ public class TestMysqlTableOperations extends TestMysql {
tableName, tableComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, loaded);
}
+ @Test
+ public void testClearCommentsWithNull() {
+ String tableName = RandomStringUtils.randomAlphabetic(16) +
"_clear_comment";
+ JdbcColumn column =
+ JdbcColumn.builder().withName("col_1").withType(INT).withComment("col
comment").build();
+ try {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ new JdbcColumn[] {column},
+ "table comment",
+ new HashMap<>(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ tableName,
+ TableChange.updateColumnComment(new String[] {"col_1"}, null),
+ TableChange.updateComment(null));
+
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(),
tableName);
+ Assertions.assertTrue(StringUtils.isEmpty(load.comment()),
load.comment());
+ Assertions.assertTrue(
+ StringUtils.isEmpty(load.columns()[0].comment()),
load.columns()[0].comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), tableName);
+ }
+ }
+
@Test
public void testCreateAllTypeTable() {
String tableName = RandomNameUtils.genRandomName("type_table_");
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
index a8292a95cb..f0328ad591 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
@@ -275,6 +275,15 @@ public class TestMysqlTableOperationsSqlGeneration {
alterSql);
}
+ @Test
+ public void testClearTableCommentWithNull() {
+ TestableMysqlTableOperations ops = new TestableMysqlTableOperations();
+
+ Assertions.assertEquals(
+ "ALTER TABLE `test_table`\nCOMMENT '';",
+ ops.alterTableSql("test_table", TableChange.updateComment(null)));
+ }
+
@Test
public void testEscapeAddColumnCommentInGeneratedSql() {
TestableMysqlTableOperations ops = new TestableMysqlTableOperations();
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
index ca5606370a..7d012a23ec 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
@@ -486,7 +486,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
private String updateCommentDefinition(
TableChange.UpdateComment updateComment, JdbcTable jdbcTable) {
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (null == StringIdentifier.fromComment(newComment)) {
// Detect and add Gravitino id.
if (StringUtils.isNotEmpty(jdbcTable.comment())) {
@@ -695,7 +695,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
private String updateColumnCommentFieldDefinition(
TableChange.UpdateColumnComment updateColumnComment, String tableName) {
- String newComment = updateColumnComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateColumnComment.getNewComment());
if (updateColumnComment.fieldName().length > 1) {
throw new
UnsupportedOperationException(POSTGRESQL_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
index ef2b1efc9a..13382aa471 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
@@ -269,6 +269,37 @@ public class TestPostgreSqlTableOperations extends
TestPostgreSql {
Assertions.assertFalse(TABLE_OPERATIONS.drop(TEST_DB_NAME, newName));
}
+ @Test
+ public void testClearCommentsWithNull() {
+ String tableName = RandomNameUtils.genRandomName("clear_comment_");
+ JdbcColumn column =
+ JdbcColumn.builder().withName("col_1").withType(INT).withComment("col
comment").build();
+ try {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ tableName,
+ new JdbcColumn[] {column},
+ "table comment",
+ new HashMap<>(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ tableName,
+ TableChange.updateColumnComment(new String[] {"col_1"}, null),
+ TableChange.updateComment(null));
+
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertTrue(StringUtils.isEmpty(load.comment()),
load.comment());
+ Assertions.assertTrue(
+ StringUtils.isEmpty(load.columns()[0].comment()),
load.columns()[0].comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, tableName);
+ }
+ }
+
@Test
public void testCreateAllTypeTable() {
String tableName = RandomNameUtils.genRandomName("type_table_");
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
index 38f439ac20..00958b7e93 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
@@ -176,6 +176,19 @@ public class TestPostgreSqlTableOperationsSqlGeneration {
alterSql);
}
+ @Test
+ public void testClearCommentsWithNull() {
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+
+ Assertions.assertEquals(
+ "COMMENT ON COLUMN \"test_table\".\"col1\" IS E'';",
+ ops.alterTableSql(
+ "test_table", TableChange.updateColumnComment(new String[]
{"col1"}, null)));
+ Assertions.assertEquals(
+ "COMMENT ON TABLE \"test_table\" IS E'';",
+ ops.alterTableSql("test_table", TableChange.updateComment(null)));
+ }
+
@Test
public void testEscapeAddColumnCommentInGeneratedSql() {
TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
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..5df1941410 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
@@ -175,7 +175,7 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
alterSql.add(updateColumnTypeFieldDefinition(updateColumnType,
lazyLoadTable));
} else if (change instanceof TableChange.UpdateComment) {
TableChange.UpdateComment updateComment = (TableChange.UpdateComment)
change;
- String newComment = updateComment.getNewComment();
+ String newComment =
StringUtils.defaultString(updateComment.getNewComment());
if (StringIdentifier.fromComment(newComment) == null) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
StringIdentifier identifier =
StringIdentifier.fromComment(lazyLoadTable.comment());
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 39763b3574..60f5338f54 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
@@ -35,6 +35,7 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
@@ -252,6 +253,37 @@ public class TestStarRocksTableOperations extends
TestStarRocks {
}
}
+ @Test
+ 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");
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 f8825ef6ae..0679214d40 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
@@ -279,6 +279,23 @@ public class TestStarRocksTableOperationsSqlGeneration {
"COMMENT = \"(From Gravitino, DO NOT EDIT: gravitino.v1.uid42) "
+ "(From Gravitino, DO NOT EDIT: gravitino.v1.uid-1)\""),
clearCommentSql);
+ Assertions.assertEquals(
+ clearCommentSql, ops.alterTableSql("test_table",
TableChange.updateComment(null)));
+ }
+
+ @Test
+ public void testClearTableCommentWithNull() {
+ TestableStarRocksTableOperations ops =
+ new TestableStarRocksTableOperations() {
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable
lazyLoadCreateTable) {
+ return JdbcTable.builder().withName(tableName).build();
+ }
+ };
+
+ String alterSql = ops.alterTableSql("test_table",
TableChange.updateComment(null));
+ Assertions.assertTrue(alterSql.contains("COMMENT = \"\""), alterSql);
}
@Test
