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 6827ca1d97 [#9694] fix (catalog-mysql) : Allow clearing column
comments in table updates (#9694) (#9881)
6827ca1d97 is described below
commit 6827ca1d977a4a1b3b6e07af82ed6e1c25ae71a8
Author: Tanay Paul <[email protected]>
AuthorDate: Wed Mar 4 17:51:53 2026 +0530
[#9694] fix (catalog-mysql) : Allow clearing column comments in table
updates (#9694) (#9881)
### What changes were proposed in this pull request?
This PR allows column comments to be cleared (set to blank or null)
during table update operations, addressing situations where column
annotations need to be removed.
### Why are the changes needed?
Previously, clearing a column comment was not possible during schema
alteration, limiting flexibility for database users or downstream tools
that manage or sanitize schema metadata.
Fix: #9694
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT and IT
---
.../mysql/integration/test/CatalogMysqlIT.java | 55 ++++++++++++++++++++++
.../gravitino/dto/requests/TableUpdateRequest.java | 3 --
.../dto/requests/TestTableUpdatesRequest.java | 33 +++++++++++++
.../testsets/jdbc-mysql/00002_alter_table.txt | 3 +-
4 files changed, 89 insertions(+), 5 deletions(-)
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
index 93aad7ea4c..8db7100339 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
@@ -896,6 +896,61 @@ public class CatalogMysqlIT extends BaseIT {
});
}
+ @Test
+ void testClearColumnComments() {
+ Column col1 = Column.of("col_with_comment1", Types.IntegerType.get(),
"original comment 1");
+ Column col2 = Column.of("col_with_comment2", Types.StringType.get(),
"original comment 2");
+ Column col3 = Column.of("col_with_comment3", Types.DateType.get(),
"original comment 3");
+ Column[] columns = new Column[] {col1, col2, col3};
+ NameIdentifier tableIdentifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName("test_clear_comments"));
+ catalog.asTableCatalog().createTable(tableIdentifier, columns, "test
table", ImmutableMap.of());
+ Table table = catalog.asTableCatalog().loadTable(tableIdentifier);
+ Assertions.assertEquals("original comment 1",
table.columns()[0].comment());
+ Assertions.assertEquals("original comment 2",
table.columns()[1].comment());
+ Assertions.assertEquals("original comment 3",
table.columns()[2].comment());
+ catalog
+ .asTableCatalog()
+ .alterTable(
+ tableIdentifier,
+ TableChange.updateColumnComment(new String[]
{"col_with_comment1"}, null));
+ catalog
+ .asTableCatalog()
+ .alterTable(
+ tableIdentifier,
+ TableChange.updateColumnComment(new String[]
{"col_with_comment2"}, ""));
+ table = catalog.asTableCatalog().loadTable(tableIdentifier);
+ Assertions.assertTrue(
+ table.columns()[0].comment() == null ||
table.columns()[0].comment().isEmpty(),
+ "Comment should be null or empty after clearing with null");
+ Assertions.assertTrue(
+ table.columns()[1].comment() == null ||
table.columns()[1].comment().isEmpty(),
+ "Comment should be null or empty after clearing with empty string");
+ Assertions.assertEquals(
+ "original comment 3", table.columns()[2].comment(), "Unchanged comment
should remain");
+ catalog.asTableCatalog().dropTable(tableIdentifier);
+ }
+
+ @Test
+ void testClearColumnComment() {
+ Column[] columns = createColumns();
+ TableCatalog tableCatalog = catalog.asTableCatalog();
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
+
+ tableCatalog.createTable(tableIdentifier, columns, table_comment,
createProperties());
+ tableCatalog.alterTable(
+ tableIdentifier,
+ TableChange.updateColumnComment(new String[] {MYSQL_COL_NAME1},
"updated_comment"));
+
+ Table table = tableCatalog.loadTable(tableIdentifier);
+ Assertions.assertEquals("updated_comment", table.columns()[0].comment());
+
+ tableCatalog.alterTable(
+ tableIdentifier, TableChange.updateColumnComment(new String[]
{MYSQL_COL_NAME1}, ""));
+ table = tableCatalog.loadTable(tableIdentifier);
+ Assertions.assertTrue(StringUtils.isBlank(table.columns()[0].comment()));
+ }
+
@Test
void testUpdateColumnDefaultValue() {
Column[] columns = createColumnsWithDefaultValue();
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
index e31e14033e..c6e58bbd98 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
@@ -623,9 +623,6 @@ public interface TableUpdateRequest extends RESTRequest {
&& fieldName.length > 0
&& Arrays.stream(fieldName).allMatch(StringUtils::isNotBlank),
"\"fieldName\" field is required and cannot be empty");
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
}
/**
diff --git
a/common/src/test/java/org/apache/gravitino/dto/requests/TestTableUpdatesRequest.java
b/common/src/test/java/org/apache/gravitino/dto/requests/TestTableUpdatesRequest.java
index 4c2c176d3b..b8e49a3a82 100644
---
a/common/src/test/java/org/apache/gravitino/dto/requests/TestTableUpdatesRequest.java
+++
b/common/src/test/java/org/apache/gravitino/dto/requests/TestTableUpdatesRequest.java
@@ -309,4 +309,37 @@ public class TestTableUpdatesRequest {
Assertions.assertEquals(
JsonUtils.objectMapper().readTree(expected),
JsonUtils.objectMapper().readTree(jsonString));
}
+
+ @Test
+ public void testUpdateColumnCommentWithEmptyString() {
+ TableUpdateRequest.UpdateTableColumnCommentRequest request =
+ new TableUpdateRequest.UpdateTableColumnCommentRequest(new String[]
{"column1"}, "");
+
+ request.validate();
+
+ Assertions.assertEquals("", request.getNewComment());
+ Assertions.assertArrayEquals(new String[] {"column1"},
request.getFieldName());
+ }
+
+ @Test
+ public void testUpdateColumnCommentWithNull() {
+ TableUpdateRequest.UpdateTableColumnCommentRequest request =
+ new TableUpdateRequest.UpdateTableColumnCommentRequest(new String[]
{"column1"}, null);
+
+ request.validate();
+
+ Assertions.assertNull(request.getNewComment());
+ Assertions.assertArrayEquals(new String[] {"column1"},
request.getFieldName());
+ }
+
+ @Test
+ public void testUpdateColumnCommentWithValidValue() {
+ TableUpdateRequest.UpdateTableColumnCommentRequest request =
+ new TableUpdateRequest.UpdateTableColumnCommentRequest(
+ new String[] {"column1"}, "This is a valid comment");
+
+ request.validate();
+
+ Assertions.assertEquals("This is a valid comment",
request.getNewComment());
+ }
}
diff --git
a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt
b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt
index b3b5366b9a..58ca30d38c 100644
---
a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt
+++
b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt
@@ -104,10 +104,9 @@ WITH (
engine = 'InnoDB'
)"
-<QUERY_FAILED> "newComment" field is required and cannot be empty
+COMMENT
DROP TABLE
DROP SCHEMA
-
