This is an automated email from the ASF dual-hosted git repository. yuqi4733 pushed a commit to branch issue_10316 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit aed8a49c98fd8c1f1f4398a8eb15f283cd9e2b90 Author: yuqi <[email protected]> AuthorDate: Mon Mar 9 21:23:53 2026 +0800 fix(clickhouse): handle pre-quoted and null string defaults Address review feedback for #10316 by unescaping doubled quotes after stripping outer quotes and rejecting null string literal defaults explicitly. Add regression tests for pre-quoted escaped strings and null defaults. Co-authored-by: Copilot <[email protected]> --- .../ClickHouseColumnDefaultValueConverter.java | 5 +- .../operations/TestClickHouseTableOperations.java | 81 ++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java index e3a76550a4..1e88841ab2 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java @@ -21,6 +21,7 @@ package org.apache.gravitino.catalog.clickhouse.converter; import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP; +import com.google.common.base.Preconditions; import java.time.LocalDate; import java.time.LocalDateTime; import org.apache.commons.lang3.StringUtils; @@ -57,6 +58,8 @@ public class ClickHouseColumnDefaultValueConverter extends JdbcColumnDefaultValu return literal.value().toString(); } else { Object value = literal.value(); + Preconditions.checkArgument( + value != null, "Null default literal value is not supported for ClickHouse."); if (value instanceof LocalDateTime) { value = ((LocalDateTime) value).format(DATE_TIME_FORMATTER); } else if (value instanceof String str @@ -65,7 +68,7 @@ public class ClickHouseColumnDefaultValueConverter extends JdbcColumnDefaultValu && str.endsWith("'")) { // The API caller may pass quoted string literals (e.g. "'active'"), but DDL generation // is responsible for adding the outer quotes. - value = str.substring(1, str.length() - 1); + value = str.substring(1, str.length() - 1).replace("''", "'"); } return String.format("'%s'", value.toString().replace("'", "''")); } 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 b1950e4965..2f9243d510 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 @@ -1138,6 +1138,87 @@ public class TestClickHouseTableOperations extends TestClickHouse { Assertions.assertTrue(sql.contains("DEFAULT 'o''reilly'")); } + @Test + void testGenerateCreateTableSqlWithPreQuotedEscapedStringDefault() { + TestableClickHouseTableOperations ops = new TestableClickHouseTableOperations(); + ops.initialize( + null, + new ClickHouseExceptionConverter(), + new ClickHouseTypeConverter(), + new ClickHouseColumnDefaultValueConverter(), + new HashMap<>()); + + JdbcColumn[] cols = + new JdbcColumn[] { + JdbcColumn.builder() + .withName("id") + .withType(Types.IntegerType.get()) + .withNullable(false) + .build(), + JdbcColumn.builder() + .withName("status") + .withType(Types.StringType.get()) + .withNullable(false) + .withDefaultValue(Literals.stringLiteral("'o''reilly'")) + .build() + }; + + String sql = + ops.buildCreateSql( + "t_status_prequoted", + cols, + null, + new HashMap<>(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + new Index[0], + ClickHouseUtils.getSortOrders("id")); + + Assertions.assertTrue(sql.contains("DEFAULT 'o''reilly'")); + Assertions.assertFalse(sql.contains("DEFAULT 'o''''reilly'")); + } + + @Test + void testGenerateCreateTableSqlWithNullStringDefaultThrows() { + TestableClickHouseTableOperations ops = new TestableClickHouseTableOperations(); + ops.initialize( + null, + new ClickHouseExceptionConverter(), + new ClickHouseTypeConverter(), + new ClickHouseColumnDefaultValueConverter(), + new HashMap<>()); + + JdbcColumn[] cols = + new JdbcColumn[] { + JdbcColumn.builder() + .withName("id") + .withType(Types.IntegerType.get()) + .withNullable(false) + .build(), + JdbcColumn.builder() + .withName("status") + .withType(Types.StringType.get()) + .withNullable(false) + .withDefaultValue(Literals.of(null, Types.StringType.get())) + .build() + }; + + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + ops.buildCreateSql( + "t_status_null_default", + cols, + null, + new HashMap<>(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + new Index[0], + ClickHouseUtils.getSortOrders("id"))); + Assertions.assertTrue(exception.getMessage().contains("Null default literal value")); + } + @Test void testParsePartitioningAndIndexExpressions() { TestableClickHouseTableOperations ops = new TestableClickHouseTableOperations();
