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 9ba4ef09d9 [Cherry-pick to branch-1.3] [#14048] fix(catalog-jdbc):
Clear comment when updating with null (#14053) (#14133)
9ba4ef09d9 is described below
commit 9ba4ef09d90d7c6a60038f31972dd06c25e782d9
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Oct 23 22:56:00 2026 +0800
[Cherry-pick to branch-1.3] [#14048] fix(catalog-jdbc): Clear comment when
updating with null (#14053) (#14133)
**Cherry-pick Information:**
- Original commit: b4097f31b2c6b30d30462ee8334c49030aff580d
- Target branch: `branch-1.3`
- Status: ✅ Conflicts resolved manually
---------
Co-authored-by: Yuhui <[email protected]>
---
.../operations/ClickHouseTableOperations.java | 8 ++-
.../operations/TestClickHouseTableOperations.java | 37 +++++++++++
.../TestClickHouseTableOperationsUnit.java | 75 ++++++++++++++++++++++
.../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, 371 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 835b6d9fa6..2f7322bda4 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
@@ -1129,7 +1129,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.
@@ -1334,6 +1334,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 4ce265908f..8967a19636 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 5dec3386a0..83b4ca9f9e 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
@@ -29,14 +29,17 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.sql.DataSource;
+import
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.ClusterConstants;
import
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.TableConstants;
import
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseColumnDefaultValueConverter;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseExceptionConverter;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.FunctionExpression;
import org.apache.gravitino.rel.expressions.NamedReference;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
@@ -53,6 +56,8 @@ import org.mockito.Mockito;
public class TestClickHouseTableOperationsUnit {
private static final class ExposedClickHouseTableOperations extends
ClickHouseTableOperations {
+ private JdbcTable table;
+
List<Index> callGetIndexes(Connection connection, String databaseName,
String tableName)
throws Exception {
return getIndexes(connection, databaseName, tableName);
@@ -91,12 +96,45 @@ public class TestClickHouseTableOperationsUnit {
Indexes.EMPTY_INDEXES,
getSortOrders("id"));
}
+
+ void setTable(JdbcTable table) {
+ this.table = table;
+ }
+
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
+ return table;
+ }
+
+ String callGenerateAlterTableSql(TableChange... changes) {
+ return generateAlterTableSql("db", "test_table", changes);
+ }
}
private ExposedClickHouseTableOperations newOps() {
return newOps(null);
}
+ private ExposedClickHouseTableOperations newAlterOps(Map<String, String>
properties) {
+ ExposedClickHouseTableOperations ops = newOps();
+ JdbcColumn idColumn =
+ JdbcColumn.builder()
+ .withName("id")
+ .withType(Types.IntegerType.get())
+ .withNullable(false)
+ .build();
+ ops.setTable(
+ JdbcTable.builder()
+ .withName("test_table")
+ .withColumns(new JdbcColumn[] {idColumn})
+ .withIndexes(Indexes.EMPTY_INDEXES)
+ .withProperties(properties)
+ .withTableOperation(null)
+ .build());
+ return ops;
+ }
+
@Test
void testCreateTableRejectsVarchar() {
JdbcColumn[] columns =
@@ -737,6 +775,43 @@ public class TestClickHouseTableOperationsUnit {
Assertions.assertTrue(exception.getMessage().contains("balanced"));
}
+ @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);
+ }
+
private RenameMocks renameMocks(String storedComment, String engineFull)
throws Exception {
DataSource dataSource = Mockito.mock(DataSource.class);
Connection connection = Mockito.mock(Connection.class);
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 c78037d291..11c03add76 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 f3e317adcd..99b0a85b1c 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
@@ -302,7 +302,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 aff7fb637b..f0a0f2d24e 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 826c3ea5e8..8ab084285d 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
@@ -922,7 +922,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);
@@ -990,7 +990,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 UnsupportedOperationException("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 20ddc08631..06716466e9 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 ce4cc80cb1..79f6d5dfbe 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
@@ -535,6 +535,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 cab50f20c4..91f6102dbe 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
@@ -389,7 +389,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 456aeb89ee..712bd55cde 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
@@ -676,6 +676,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 0b9f51a1a0..4e86865823 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
@@ -216,6 +216,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 75aa318048..9ea122c3e4 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 c241b41904..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
@@ -178,7 +178,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 68afce08bb..114f8e50f8 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
@@ -298,6 +298,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
