This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 e8f5c73805 [Cherry-pick to branch-1.3] [#11880] fix(clickhouse):
preserve SETTINGS clause on table round-trip (#11885) (#12000)
e8f5c73805 is described below
commit e8f5c73805756f2134c255070504b0676eee5ee6
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Jul 13 19:26:05 2026 +0800
[Cherry-pick to branch-1.3] [#11880] fix(clickhouse): preserve SETTINGS
clause on table round-trip (#11885) (#12000)
**Cherry-pick Information:**
- Original commit: 4894435f7a396c9f96b86bfbc6368bd8a6b2f56a
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Signed-off-by: jiangxt2 <[email protected]>
Co-authored-by: StormSpirit <[email protected]>
---
.../operations/ClickHouseTableOperations.java | 49 +++++++-
.../integration/test/CatalogClickHouseIT.java | 140 +++++++++++++++++++++
.../test/service/ClickHouseService.java | 12 ++
.../operations/TestClickHouseTableOperations.java | 42 +++++++
4 files changed, 240 insertions(+), 3 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 6eb48dcea0..118eed0989 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
@@ -87,6 +87,8 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
private static final Pattern PARTITION_BY_PATTERN =
Pattern.compile(
"(?is)\\bPARTITION\\s+BY\\s*(.+?)(?=\\bORDER\\s+BY\\b|\\bPRIMARY\\s+KEY\\b|\\bSAMPLE\\s+BY\\b|\\bTTL\\b|\\bSETTINGS\\b|\\bCOMMENT\\b|$)");
+ private static final Pattern SETTINGS_PATTERN =
+ Pattern.compile("(?is)\\bSETTINGS\\s+(.+?)(?=\\bCOMMENT\\b|$)");
private static final Pattern DISTRIBUTED_ENGINE_PATTERN =
Pattern.compile(
"(?i)^Distributed\\(([^,]+),\\s*([^,]+),\\s*([^,]+),\\s*(.+)\\)$",
Pattern.DOTALL);
@@ -191,6 +193,11 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
appendPartitionClause(partitioning, sqlBuilder, engine);
+ // Add setting clause before COMMENT; ClickHouse 24.8 rejects SETTINGS
that follow COMMENT
+ // (all settings become UNKNOWN_SETTING when preceded by a COMMENT clause).
+ // This matches the order in SHOW CREATE TABLE output: SETTINGS ...
COMMENT '...'.
+ appendTableProperties(notNullProperties, sqlBuilder);
+
// Add table comment; embed cluster name so it can be recovered at
DROP/ALTER time.
// ClickHouse does not persist ON CLUSTER in SHOW CREATE TABLE (see
ClickHouseClusterUtils).
String storedComment =
@@ -202,9 +209,6 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
sqlBuilder.append(" COMMENT
'%s'".formatted(escapeSingleQuotes(storedComment)));
}
- // Add setting clause if specified, clickhouse only supports predefine
settings
- appendTableProperties(notNullProperties, sqlBuilder);
-
// Return the generated SQL statement
String result = sqlBuilder.toString();
@@ -616,6 +620,15 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
jdbcTableBuilder.withDistribution(distribution);
Map<String, String> tableProperties = getTableProperties(connection,
tableName);
+ // Merge SETTINGS parsed from SHOW CREATE TABLE into table properties.
+ // SHOW CREATE TABLE is the authoritative source for SETTINGS; it takes
precedence
+ // over any settings.* keys that might exist in system.tables (though
getTableProperties()
+ // currently does not read SETTINGS from system.tables, so no overlap
occurs in practice).
+ if (!metadata.settings.isEmpty()) {
+ Map<String, String> merged = new HashMap<>(tableProperties);
+ merged.putAll(metadata.settings);
+ tableProperties = Collections.unmodifiableMap(merged);
+ }
jdbcTableBuilder.withProperties(tableProperties);
correctJdbcTableFields(connection, databaseName, tableName,
jdbcTableBuilder);
@@ -1121,14 +1134,43 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
metadata.partitioning = parsePartitioning(partitionMatcher.group(1));
}
+ Matcher settingsMatcher = SETTINGS_PATTERN.matcher(createSql);
+ if (settingsMatcher.find()) {
+ metadata.settings = parseSettingsClause(settingsMatcher.group(1));
+ }
+
return metadata;
}
+ // Parses "key1 = val1, key2 = val2" from a SETTINGS clause.
+ // Keys are prefixed with "settings." to match the write path convention in
+ // appendTableProperties(). ClickHouse SETTINGS values are scalar (UInt64,
Bool,
+ // String, Enum) — arrays or nested structures are not valid SETTINGS values,
+ // so splitting by comma is safe.
+ private static Map<String, String> parseSettingsClause(String settingsStr) {
+ Map<String, String> settings = new HashMap<>();
+ for (String pair : settingsStr.split(",")) {
+ String trimmed = pair.trim();
+ int eqIdx = trimmed.indexOf('=');
+ if (eqIdx > 0) {
+ String key = trimmed.substring(0, eqIdx).trim();
+ String value = trimmed.substring(eqIdx + 1).trim();
+ settings.put(TableConstants.SETTINGS_PREFIX + key, value);
+ }
+ }
+ return settings;
+ }
+
@VisibleForTesting
SortOrder[] parseSortOrdersFromCreateSql(String createSql) {
return parseCreateStatement(createSql).sortOrders;
}
+ @VisibleForTesting
+ Map<String, String> parseSettingsFromCreateSql(String createSql) {
+ return parseCreateStatement(createSql).settings;
+ }
+
private ShowCreateTableMetadata parseShowCreateTable(Connection connection,
String tableName)
throws SQLException {
String createSql = parseShowCreateTableSql(connection, tableName);
@@ -1248,6 +1290,7 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
private static final class ShowCreateTableMetadata {
private Transform[] partitioning = Transforms.EMPTY_TRANSFORM;
private SortOrder[] sortOrders = SortOrders.NONE;
+ private Map<String, String> settings = Collections.emptyMap();
}
@VisibleForTesting
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 eeb31d5731..4da92f33e1 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
@@ -45,6 +45,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.Schema;
import org.apache.gravitino.SupportsSchemas;
import org.apache.gravitino.auth.AuthConstants;
+import
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.TableConstants;
import
org.apache.gravitino.catalog.clickhouse.integration.test.service.ClickHouseService;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.client.GravitinoMetalake;
@@ -2261,4 +2262,143 @@ public class CatalogClickHouseIT extends BaseIT {
loadCatalog.asSchemas().dropSchema("test", true);
metalake.dropCatalog(testCatalogName, true);
}
+
+ @Test
+ void testLoadTableWithSettingsFromNativeSql() {
+ String name = GravitinoITUtils.genRandomName("settings_native");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` (id Int32) ENGINE = MergeTree ORDER BY id"
+ + " SETTINGS index_granularity = 2048",
+ schemaName, name));
+
+ Table loaded =
catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName, name));
+ Map<String, String> props = loaded.properties();
+ Assertions.assertEquals(
+ "2048", props.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ // Ensure no spurious keys from the SETTINGS parsing
+ long settingsCount =
+ props.keySet().stream().filter(k ->
k.startsWith(TableConstants.SETTINGS_PREFIX)).count();
+ Assertions.assertEquals(1, settingsCount);
+ }
+
+ @Test
+ void testCreateTableWithSettingsViaGravitinoApi() {
+ String name = GravitinoITUtils.genRandomName("settings_api");
+ NameIdentifier ident = NameIdentifier.of(schemaName, name);
+ Column[] cols =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+
+ Map<String, String> properties = createProperties();
+ properties.put(TableConstants.SETTINGS_PREFIX + "index_granularity",
"2048");
+ properties.put(TableConstants.SETTINGS_PREFIX + "min_bytes_for_wide_part",
"0");
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident, cols, "settings roundtrip", properties, Distributions.NONE,
getSortOrders("id"));
+
+ // Verify Gravitino API round-trip
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertEquals(
+ "2048", loadedProps.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ Assertions.assertEquals(
+ "0", loadedProps.get(TableConstants.SETTINGS_PREFIX +
"min_bytes_for_wide_part"));
+
+ // Verify DDL-level round-trip: SHOW CREATE TABLE should contain SETTINGS
clause
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName, name));
+ Assertions.assertNotNull(createSql);
+ Assertions.assertTrue(
+ createSql.contains("SETTINGS"),
+ "SHOW CREATE TABLE output should contain SETTINGS clause: " +
createSql);
+ Assertions.assertTrue(
+ createSql.contains("index_granularity = 2048"),
+ "SHOW CREATE TABLE output should contain index_granularity = 2048: " +
createSql);
+ }
+
+ @Test
+ void testLoadTableWithMultipleSettingsAndComment() {
+ String name = GravitinoITUtils.genRandomName("settings_multi");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` (id Int32) ENGINE = MergeTree ORDER BY id"
+ + " SETTINGS index_granularity = 2048, min_bytes_for_wide_part
= 0"
+ + " COMMENT 'test comment'",
+ schemaName, name));
+
+ Table loaded =
catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName, name));
+ Map<String, String> props = loaded.properties();
+ Assertions.assertEquals(
+ "2048", props.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ Assertions.assertEquals(
+ "0", props.get(TableConstants.SETTINGS_PREFIX +
"min_bytes_for_wide_part"));
+ Assertions.assertEquals("test comment", loaded.comment());
+ }
+
+ @Test
+ void testCreateTableWithSettingsAndCommentViaGravitinoApi() {
+ String name = GravitinoITUtils.genRandomName("settings_comment_api");
+ NameIdentifier ident = NameIdentifier.of(schemaName, name);
+ Column[] cols =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+
+ Map<String, String> properties = createProperties();
+ properties.put(TableConstants.SETTINGS_PREFIX + "index_granularity",
"2048");
+ properties.put(TableConstants.SETTINGS_PREFIX + "min_bytes_for_wide_part",
"0");
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident,
+ cols,
+ "settings and comment roundtrip",
+ properties,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ // Verify Gravitino API round-trip
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertEquals(
+ "2048", loadedProps.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ Assertions.assertEquals(
+ "0", loadedProps.get(TableConstants.SETTINGS_PREFIX +
"min_bytes_for_wide_part"));
+ Assertions.assertEquals("settings and comment roundtrip",
loaded.comment());
+
+ // Verify DDL-level round-trip
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName, name));
+ Assertions.assertNotNull(createSql);
+ Assertions.assertTrue(
+ createSql.contains("SETTINGS"),
+ "SHOW CREATE TABLE output should contain SETTINGS clause: " +
createSql);
+ Assertions.assertTrue(
+ createSql.contains("index_granularity = 2048"),
+ "SHOW CREATE TABLE output should contain index_granularity = 2048: " +
createSql);
+ }
+
+ @Test
+ void testLoadTableWithoutSettings() {
+ String name = GravitinoITUtils.genRandomName("no_settings");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` (id Int32) ENGINE = MergeTree ORDER BY
id", schemaName, name));
+
+ Table loaded =
catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName, name));
+ long settingsCount =
+ loaded.properties().keySet().stream()
+ .filter(k -> k.startsWith(TableConstants.SETTINGS_PREFIX))
+ .count();
+ // ClickHouse 24.8 always includes default SETTINGS (index_granularity =
8192) in
+ // SHOW CREATE TABLE output, so one settings key is expected even without
explicit SETTINGS.
+ Assertions.assertEquals(1, settingsCount);
+ }
}
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/service/ClickHouseService.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/service/ClickHouseService.java
index d71d920801..e30523853e 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/service/ClickHouseService.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/service/ClickHouseService.java
@@ -101,6 +101,18 @@ public class ClickHouseService {
}
}
+ public String executeQueryForResult(String sql) {
+ try (Statement statement = connection.createStatement();
+ ResultSet resultSet = statement.executeQuery(sql)) {
+ if (resultSet.next()) {
+ return resultSet.getString(1);
+ }
+ return null;
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
public void close() {
try {
connection.close();
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 93c928a539..946a1a3027 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
@@ -1194,6 +1194,44 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Assertions.assertEquals("id", ((NamedReference)
sortOrders[0].expression()).fieldName()[0]);
}
+ @Test
+ void testParseSettingsFromCreateSql() {
+ TestableClickHouseTableOperations ops = new
TestableClickHouseTableOperations();
+
+ // Single setting
+ String sql1 =
+ "CREATE TABLE t1 (id Int32) ENGINE = MergeTree ORDER BY id SETTINGS
index_granularity = 4096";
+ Map<String, String> settings1 = ops.parseSettings(sql1);
+ Assertions.assertEquals(1, settings1.size());
+ Assertions.assertEquals(
+ "4096", settings1.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+
+ // Multiple settings
+ String sql2 =
+ "CREATE TABLE t2 (id Int32) ENGINE = MergeTree ORDER BY id"
+ + " SETTINGS index_granularity = 4096, min_bytes_for_wide_part =
0";
+ Map<String, String> settings2 = ops.parseSettings(sql2);
+ Assertions.assertEquals(2, settings2.size());
+ Assertions.assertEquals(
+ "4096", settings2.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ Assertions.assertEquals(
+ "0", settings2.get(TableConstants.SETTINGS_PREFIX +
"min_bytes_for_wide_part"));
+
+ // No SETTINGS clause
+ String sql3 = "CREATE TABLE t3 (id Int32) ENGINE = MergeTree ORDER BY id";
+ Map<String, String> settings3 = ops.parseSettings(sql3);
+ Assertions.assertTrue(settings3.isEmpty());
+
+ // SETTINGS with COMMENT after
+ String sql4 =
+ "CREATE TABLE t4 (id Int32) ENGINE = MergeTree ORDER BY id"
+ + " SETTINGS index_granularity = 8192 COMMENT 'test'";
+ Map<String, String> settings4 = ops.parseSettings(sql4);
+ Assertions.assertEquals(1, settings4.size());
+ Assertions.assertEquals(
+ "8192", settings4.get(TableConstants.SETTINGS_PREFIX +
"index_granularity"));
+ }
+
private static final class TestableClickHouseTableOperations extends
ClickHouseTableOperations {
String buildCreateSql(
String tableName,
@@ -1211,6 +1249,10 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
SortOrder[] parseSortOrders(String createSql) {
return parseSortOrdersFromCreateSql(createSql);
}
+
+ Map<String, String> parseSettings(String createSql) {
+ return parseSettingsFromCreateSql(createSql);
+ }
}
@Test
