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 c1449a3b4c [#10316] fix(catalog-jdbc-clickhouse): normalize quoted
string defaults (#10318)
c1449a3b4c is described below
commit c1449a3b4cfedb0e181a11208a90456758a9cef6
Author: Qi Yu <[email protected]>
AuthorDate: Wed May 6 17:44:01 2026 +0800
[#10316] fix(catalog-jdbc-clickhouse): normalize quoted string defaults
(#10318)
### What changes were proposed in this pull request?
This PR fixes ClickHouse string default value SQL rendering when
creating or altering tables via API.
Changes:
- Normalize quoted string literals in
ClickHouseColumnDefaultValueConverter#fromGravitino.
- If input is already wrapped by outer single quotes (for example,
'active'), strip the outer quotes before SQL wrapping.
- Escape inner single quotes safely (' -> '').
- Add regression tests:
-
TestClickHouseTableOperations#testGenerateCreateTableSqlWithQuotedStringDefault
-
TestClickHouseTableOperations#testGenerateCreateTableSqlEscapesStringDefaultQuotes
- CatalogClickHouseIT#testCreateTableWithQuotedStringDefaultLiteral
### Why are the changes needed?
When API clients pass a string default like 'active' with surrounding
quotes in the value, generated DDL became DEFAULT ''active'' instead of
DEFAULT 'active'.
Fix: #10316
### Does this PR introduce _any_ user-facing change?
Yes.
Creating/altering ClickHouse tables with quoted string defaults now
generates correct SQL defaults and preserves expected values.
### How was this patch tested?
Ran:
./gradlew --no-daemon -p catalogs-contrib/catalog-jdbc-clickhouse test
--tests
org.apache.gravitino.catalog.clickhouse.operations.TestClickHouseTableOperations
--tests
org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT
All targeted tests passed.
---------
Co-authored-by: Copilot <[email protected]>
---
.../ClickHouseColumnDefaultValueConverter.java | 14 +-
.../operations/ClickHouseClusterUtils.java | 4 +
.../integration/test/CatalogClickHouseIT.java | 95 +++++++++++
.../operations/TestClickHouseTableOperations.java | 180 +++++++++++++++++++--
.../catalog-lakehouse-iceberg/build.gradle.kts | 6 +-
5 files changed, 285 insertions(+), 14 deletions(-)
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 236b498172..a9458b1289 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,9 +21,11 @@ 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;
+import
org.apache.gravitino.catalog.clickhouse.operations.ClickHouseClusterUtils;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
import org.apache.gravitino.rel.expressions.Expression;
@@ -57,10 +59,20 @@ 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
+ && str.length() >= 2
+ && str.startsWith("'")
+ && str.endsWith("'")) {
+ // The API caller may pass quoted string literals (e.g. "'active'"),
but DDL generation
+ // is responsible for adding the outer quotes.
+ value = ClickHouseClusterUtils.unescapeSingleQuotes(str.substring(1,
str.length() - 1));
}
- return String.format("'%s'", value);
+
+ return String.format("'%s'",
ClickHouseClusterUtils.escapeSingleQuotes(value.toString()));
}
}
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseClusterUtils.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseClusterUtils.java
index 830c032f70..b225102650 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseClusterUtils.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseClusterUtils.java
@@ -166,4 +166,8 @@ public final class ClickHouseClusterUtils {
public static String escapeSingleQuotes(String text) {
return text.replace("'", "''");
}
+
+ public static String unescapeSingleQuotes(String text) {
+ return text.replace("''", "'");
+ }
}
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java
index 794189d0ff..ab3d95b870 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java
@@ -654,6 +654,101 @@ public class CatalogClickHouseIT extends BaseIT {
Literals.stringLiteral("now()"),
createdTable.columns()[4].defaultValue());
}
+ @Test
+ void testCreateTableWithCommonTypeLiteralDefaults() {
+ String name = GravitinoITUtils.genRandomName("default_literal_table");
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, name);
+
+ Column[] columns =
+ new Column[] {
+ Column.of(
+ "int_col", Types.IntegerType.get(), "int", false, false,
Literals.integerLiteral(7)),
+ Column.of(
+ "double_col",
+ Types.DoubleType.get(),
+ "double",
+ false,
+ false,
+ Literals.doubleLiteral(123.45)),
+ Column.of(
+ "string_col",
+ Types.VarCharType.of(255),
+ "string",
+ false,
+ false,
+ Literals.stringLiteral("hello")),
+ Column.of(
+ "date_col",
+ Types.DateType.get(),
+ "date",
+ false,
+ false,
+ Literals.dateLiteral(LocalDate.of(2024, 4, 1))),
+ Column.of(
+ "decimal_col",
+ Types.DecimalType.of(5, 2),
+ "decimal",
+ false,
+ false,
+ Literals.decimalLiteral(Decimal.of("9.99", 5, 2)))
+ };
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ tableIdentifier,
+ columns,
+ table_comment,
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ getSortOrders("int_col"));
+
+ Table loadedTable = catalog.asTableCatalog().loadTable(tableIdentifier);
+ Assertions.assertEquals(Literals.integerLiteral(7),
loadedTable.columns()[0].defaultValue());
+ Assertions.assertEquals(
+ Literals.doubleLiteral(123.45),
loadedTable.columns()[1].defaultValue());
+ Assertions.assertEquals(
+ Literals.stringLiteral("hello"),
loadedTable.columns()[2].defaultValue());
+ Assertions.assertEquals(
+ Literals.dateLiteral(LocalDate.of(2024, 4, 1)),
loadedTable.columns()[3].defaultValue());
+ Assertions.assertEquals(
+ Literals.decimalLiteral(Decimal.of("9.99", 5, 2)),
loadedTable.columns()[4].defaultValue());
+ }
+
+ @Test
+ void testCreateTableWithQuotedStringDefaultLiteral() {
+ String name = GravitinoITUtils.genRandomName("quoted_default_table");
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, name);
+
+ Column[] columns =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of(
+ "status",
+ Types.StringType.get(),
+ "Status",
+ false,
+ false,
+ Literals.stringLiteral("'active'"))
+ };
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ tableIdentifier,
+ columns,
+ table_comment,
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ Table loadedTable = catalog.asTableCatalog().loadTable(tableIdentifier);
+ Assertions.assertEquals(
+ Literals.stringLiteral("active"),
loadedTable.columns()[1].defaultValue());
+ }
+
@Test
// see https://dev.clickhouse.com/doc/refman/8.0/en/data-type-defaults.html
@EnabledIf("supportColumnDefaultValueExpression")
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 a7cc763e30..47188929e7 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
@@ -32,7 +32,6 @@ import java.util.List;
import java.util.Map;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringUtils;
-import org.apache.gravitino.catalog.clickhouse.ClickHouseConstants;
import
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.TableConstants;
import
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata;
import org.apache.gravitino.catalog.clickhouse.ClickHouseUtils;
@@ -894,9 +893,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Index[] indexes =
new Index[] {
Indexes.of(
- Index.IndexType.PRIMARY_KEY,
- Indexes.DEFAULT_PRIMARY_KEY_NAME,
- new String[][] {{"c1"}})
+ IndexType.PRIMARY_KEY, Indexes.DEFAULT_PRIMARY_KEY_NAME, new
String[][] {{"c1"}})
};
Map<String, String> propsWithPartition = new HashMap<>();
@@ -988,7 +985,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Map<String, String> props = new HashMap<>();
props.put(
TableConstants.ENGINE_UPPER,
ClickHouseTablePropertiesMetadata.ENGINE.MERGETREE.getValue());
- props.put(ClickHouseConstants.TableConstants.SETTINGS_PREFIX +
"max_threads", "8");
+ props.put(TableConstants.SETTINGS_PREFIX + "max_threads", "8");
String sql =
ops.buildCreateSql(
"t1",
@@ -1182,8 +1179,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
TableChange.updateColumnPosition(new String[] {"c1"},
TableChange.ColumnPosition.first()),
TableChange.deleteColumn(new String[] {"c3"}, false),
TableChange.updateColumnNullability(new String[] {"c2"}, false),
- TableChange.addIndex(
- Index.IndexType.DATA_SKIPPING_MINMAX, "idx2", new String[][]
{{"c2"}}),
+ TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "idx2", new
String[][] {{"c2"}}),
TableChange.deleteIndex("idx1", false),
TableChange.renameColumn(new String[] {"c2"}, "c2_new"),
TableChange.updateComment("new_table_comment")
@@ -1263,7 +1259,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
"tbl",
new TableChange[] {
TableChange.addIndex(
- Index.IndexType.DATA_SKIPPING_MINMAX, "idx_new", new
String[][] {{"c2"}})
+ IndexType.DATA_SKIPPING_MINMAX, "idx_new", new String[][]
{{"c2"}})
});
Assertions.assertTrue(minMaxSql.contains("ADD INDEX `idx_new` `c2` TYPE
minmax GRANULARITY 1"));
@@ -1273,7 +1269,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
"tbl",
new TableChange[] {
TableChange.addIndex(
- Index.IndexType.DATA_SKIPPING_BLOOM_FILTER, "idx_bf", new
String[][] {{"c2"}})
+ IndexType.DATA_SKIPPING_BLOOM_FILTER, "idx_bf", new
String[][] {{"c2"}})
});
Assertions.assertTrue(
bloomSql.contains("ADD INDEX `idx_bf` `c2` TYPE bloom_filter
GRANULARITY 3"));
@@ -1286,7 +1282,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
"tbl",
new TableChange[] {
TableChange.addIndex(
- Index.IndexType.DATA_SKIPPING_MINMAX, "idx1", new
String[][] {{"c2"}})
+ IndexType.DATA_SKIPPING_MINMAX, "idx1", new String[][]
{{"c2"}})
}));
Assertions.assertThrows(
@@ -1296,8 +1292,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
"db",
"tbl",
new TableChange[] {
- TableChange.addIndex(
- Index.IndexType.PRIMARY_KEY, "pk_new", new String[][]
{{"c1"}})
+ TableChange.addIndex(IndexType.PRIMARY_KEY, "pk_new", new
String[][] {{"c1"}})
}));
}
@@ -1356,6 +1351,167 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
() -> ops.buildAlterSql("db", "tbl", new TableChange[]
{TableChange.removeProperty("k")}));
}
+ @Test
+ void testGenerateCreateTableSqlWithQuotedStringDefault() {
+ 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)
+ .withComment("Status")
+ .withDefaultValue(Literals.stringLiteral("'active'"))
+ .build()
+ };
+
+ String sql =
+ ops.buildCreateSql(
+ "t_status",
+ cols,
+ null,
+ new HashMap<>(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ new Index[0],
+ ClickHouseUtils.getSortOrders("id"));
+
+ Assertions.assertTrue(sql.contains("`status` String DEFAULT 'active'
COMMENT 'Status'"));
+ Assertions.assertFalse(sql.contains("DEFAULT ''active''"));
+ }
+
+ @Test
+ void testGenerateCreateTableSqlEscapesStringDefaultQuotes() {
+ 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_quote",
+ cols,
+ null,
+ new HashMap<>(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ new Index[0],
+ ClickHouseUtils.getSortOrders("id"));
+
+ 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"));
+ }
+
private static JdbcTable buildStubTable() {
JdbcColumn c1 =
JdbcColumn.builder()
diff --git a/catalogs/catalog-lakehouse-iceberg/build.gradle.kts
b/catalogs/catalog-lakehouse-iceberg/build.gradle.kts
index f3a69c10fd..5537715581 100644
--- a/catalogs/catalog-lakehouse-iceberg/build.gradle.kts
+++ b/catalogs/catalog-lakehouse-iceberg/build.gradle.kts
@@ -38,7 +38,11 @@ dependencies {
compileOnly(libs.lombok)
implementation(project(":catalogs:catalog-common"))
- implementation(project(":iceberg:iceberg-common"))
+ implementation(project(":iceberg:iceberg-common")) {
+ exclude(module = "api")
+ exclude(module = "core")
+ exclude(module = "common")
+ }
implementation(libs.asm)
implementation(libs.bundles.iceberg)
