This is an automated email from the ASF dual-hosted git repository. github-bot pushed a commit to branch cherry-pick-6827ca1d-to-branch-1.2 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit de9f22113af5cfaaf954265ca945ffdd4cad8cdf 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 576cef39eb..82717dd599 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 11804ae7e5..cb4dcfd819 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 -
