This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 59ca784240 [#12273] fix(clickhouse): preserve MergeTree engine
parameters through create-load round-trip (#12274)
59ca784240 is described below
commit 59ca7842408a6865ddd10a8fcfd8f3022429e4d8
Author: StormSpirit <[email protected]>
AuthorDate: Thu Aug 27 17:34:54 2026 +0800
[#12273] fix(clickhouse): preserve MergeTree engine parameters through
create-load round-trip (#12274)
### What changes were proposed in this pull request?
Read the `engine_full` column from `system.tables` (already fetched via
`SELECT *`) to extract engine parameters for non-Distributed
MergeTree-family engines, store them as a new `engine_parameters` table
property, and include them when generating `ENGINE = ...` clauses in
`CREATE TABLE` DDL.
### Why are the changes needed?
The `engine` column in `system.tables` only returns the engine name
(e.g. `ReplacingMergeTree`) without parameters. After a create→load
round-trip (or loading a pre-existing table), the parameters in
parentheses are silently dropped. `ReplacingMergeTree(ts)` becomes plain
`ReplacingMergeTree` — the deduplication version column is lost,
changing the table's correctness semantics.
This affects five MergeTree-family engines:
- `ReplacingMergeTree` — version column for dedup
- `CollapsingMergeTree` — sign column for collapse
- `VersionedCollapsingMergeTree` — sign + version for versioned collapse
- `SummingMergeTree` — columns (including nested-paren tuples) for
auto-sum
- `GraphiteMergeTree` — graphite rollup config
The `engine_full` column already contains the full engine DDL and is
already parsed for `Distributed` engines; extending this to other
engines reuses the existing query path.
Fix: #12273
### Does this PR introduce _any_ user-facing change?
A new optional table property `engine_parameters` is added for
ClickHouse tables. Users can set it when creating tables (e.g.
`engine_parameters = "ts"` for `ReplacingMergeTree(ts)`), and it will
appear in `table.properties()` for loaded tables that have engine
parameters.
No existing property keys are changed or removed.
### How was this patch tested?
- Unit tests: 9 test cases covering single-param, multi-param,
nested-paren (`SummingMergeTree((a, b))`), no-param, blank/null input,
engine name mismatch, `GraphiteMergeTree` quote preservation, and
`AggregatingMergeTree` no-param.
- Docker IT: 5 round-trip tests on a real ClickHouse container covering
`ReplacingMergeTree`, `SummingMergeTree` (nested parens),
`VersionedCollapsingMergeTree` (multi-param), `CollapsingMergeTree`
(load from existing table), and `MergeTree` (no params). Each test
verifies both the Gravitino property value and the `SHOW CREATE TABLE`
output.
---------
Signed-off-by: jiangxt2 <[email protected]>
---
.../catalog/clickhouse/ClickHouseConstants.java | 3 +
.../ClickHouseTablePropertiesMetadata.java | 12 +
.../operations/ClickHouseTableOperations.java | 246 +++++++++++++++----
.../integration/test/CatalogClickHouseIT.java | 186 +++++++++++++++
.../TestClickHouseTableOperationsUnit.java | 264 +++++++++++++++++++++
5 files changed, 670 insertions(+), 41 deletions(-)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseConstants.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseConstants.java
index b946edcbc7..473ac35d3d 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseConstants.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseConstants.java
@@ -48,6 +48,9 @@ public class ClickHouseConstants {
public static final String ENGINE_UPPER = "ENGINE";
public static final String SETTINGS_PREFIX = "settings.";
public static final String GRAPHITE_CONFIG = "graphite.config";
+
+ /** Parameters for supported parameterized MergeTree engines, without
outer parentheses. */
+ public static final String ENGINE_PARAMETERS = "engine_parameters";
}
public static final class IndexConstants {
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseTablePropertiesMetadata.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseTablePropertiesMetadata.java
index 1734ae63c9..50ab7af396 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseTablePropertiesMetadata.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseTablePropertiesMetadata.java
@@ -99,6 +99,17 @@ public class ClickHouseTablePropertiesMetadata extends
JdbcTablePropertiesMetada
"",
false);
+ /** Parameters accepted by the supported parameterized MergeTree engines. */
+ public static final PropertyEntry<String> ENGINE_PARAMETERS_PROPERTY_ENTRY =
+ stringOptionalPropertyEntry(
+ TableConstants.ENGINE_PARAMETERS,
+ "Parameters supplied when creating, and restored when loading,
ReplacingMergeTree, "
+ + "SummingMergeTree, CollapsingMergeTree, and
VersionedCollapsingMergeTree tables. "
+ + "Use graphite.config for GraphiteMergeTree.",
+ false,
+ "",
+ false);
+
private static final Map<String, PropertyEntry<?>> PROPERTIES_METADATA =
createPropertiesMetadata();
@@ -125,6 +136,7 @@ public class ClickHouseTablePropertiesMetadata extends
JdbcTablePropertiesMetada
CLUSTER_REMOTE_DATABASE_PROPERTY_ENTRY.getName(),
CLUSTER_REMOTE_DATABASE_PROPERTY_ENTRY);
map.put(CLUSTER_REMOTE_TABLE_PROPERTY_ENTRY.getName(),
CLUSTER_REMOTE_TABLE_PROPERTY_ENTRY);
map.put(CLUSTER_SHARDING_KEY_PROPERTY_ENTRY.getName(),
CLUSTER_SHARDING_KEY_PROPERTY_ENTRY);
+ map.put(ENGINE_PARAMETERS_PROPERTY_ENTRY.getName(),
ENGINE_PARAMETERS_PROPERTY_ENTRY);
return Collections.unmodifiableMap(map);
}
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 7da8809e1f..a5be1b0753 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
@@ -40,14 +40,17 @@ import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.MapUtils;
import org.apache.commons.lang3.ArrayUtils;
@@ -92,6 +95,14 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
/** Default GRANULARITY for data skipping indexes, matching ClickHouse's own
default. */
private static final long DEFAULT_INDEX_GRANULARITY = 1;
+ private static final Set<ENGINE> GENERIC_ENGINE_PARAMETER_ENGINES =
+ Collections.unmodifiableSet(
+ EnumSet.of(
+ ENGINE.REPLACINGMERGETREE,
+ ENGINE.SUMMINGMERGETREE,
+ ENGINE.COLLAPSINGMERGETREE,
+ ENGINE.VERSIONEDCOLLAPSINGMERGETREE));
+
private static final Pattern ORDER_BY_PATTERN =
Pattern.compile(
"(?is)\\bORDER\\s+BY\\s*(.+?)(?=\\bPARTITION\\s+BY\\b|\\bPRIMARY\\s+KEY\\b|\\bSAMPLE\\s+BY\\b|\\bTTL\\b|\\bSETTINGS\\b|\\bCOMMENT\\b|$)");
@@ -441,6 +452,9 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
}
}
+ String engineParams =
StringUtils.trim(properties.get(TableConstants.ENGINE_PARAMETERS));
+ validateEngineParameters(engine, engineParams);
+
if (engine == ENGINE.DISTRIBUTED) {
handleDistributeTable(properties, sqlBuilder, columns);
return engine;
@@ -452,13 +466,16 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
StringUtils.isNotBlank(config),
"GraphiteMergeTree requires '%s' property referencing a
<graphite_rollup> config element",
TableConstants.GRAPHITE_CONFIG);
- // Escape single quotes to prevent SQL injection
- String escapedConfig = config.replace("'", "''");
+ String escapedConfig = JdbcConnectorUtils.escapeSqlLiteral(config, '\'');
sqlBuilder.append("\n ENGINE =
GraphiteMergeTree('%s')".formatted(escapedConfig));
return engine;
}
- sqlBuilder.append("\n ENGINE = %s".formatted(engine.getValue()));
+ if (StringUtils.isNotBlank(engineParams)) {
+ sqlBuilder.append("\n ENGINE = %s(%s)".formatted(engine.getValue(),
engineParams));
+ } else {
+ sqlBuilder.append("\n ENGINE = %s".formatted(engine.getValue()));
+ }
return engine;
}
@@ -687,44 +704,53 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
while (resultSet.next()) {
String name = resultSet.getString("name");
if (Objects.equals(name, tableName)) {
- return Collections.unmodifiableMap(
- new HashMap<String, String>() {
- {
- // Extract cluster name embedded in the COMMENT at create
time.
- // SHOW CREATE TABLE does not include ON CLUSTER (see
ClickHouseClusterUtils).
- String storedComment = resultSet.getString(COMMENT);
- String clusterName =
-
ClickHouseClusterUtils.extractClusterFromComment(storedComment);
- put(COMMENT,
ClickHouseClusterUtils.stripClusterMetadata(storedComment));
- String engine = resultSet.getString(CLICKHOUSE_ENGINE_KEY);
- put(GRAVITINO_ENGINE_KEY, engine);
- if (StringUtils.isNotBlank(clusterName)) {
- put(ClusterConstants.ON_CLUSTER, String.valueOf(true));
- put(ClusterConstants.CLUSTER_NAME, clusterName);
- } else {
- put(ClusterConstants.ON_CLUSTER, String.valueOf(false));
- }
-
- if (StringUtils.equalsIgnoreCase(engine,
ENGINE.DISTRIBUTED.getValue())) {
- String engineFull = resultSet.getString("engine_full");
- Matcher distributedEngineMatcher =
-
DISTRIBUTED_ENGINE_PATTERN.matcher(StringUtils.trimToEmpty(engineFull));
- if (distributedEngineMatcher.matches()) {
- String distributedClusterName =
unquote(distributedEngineMatcher.group(1));
- put(ClusterConstants.CLUSTER_NAME,
distributedClusterName);
- put(
- DistributedTableConstants.REMOTE_DATABASE,
- unquote(distributedEngineMatcher.group(2)));
- put(
- DistributedTableConstants.REMOTE_TABLE,
- unquote(distributedEngineMatcher.group(3)));
- put(
- DistributedTableConstants.SHARDING_KEY,
-
StringUtils.trim(distributedEngineMatcher.group(4)));
- }
- }
- }
- });
+ Map<String, String> tableProperties = new HashMap<>();
+
+ // Extract cluster name embedded in the COMMENT at create time.
+ // SHOW CREATE TABLE does not include ON CLUSTER (see
ClickHouseClusterUtils).
+ String storedComment = resultSet.getString(COMMENT);
+ String clusterName =
ClickHouseClusterUtils.extractClusterFromComment(storedComment);
+ tableProperties.put(
+ COMMENT,
ClickHouseClusterUtils.stripClusterMetadata(storedComment));
+ String engine = resultSet.getString(CLICKHOUSE_ENGINE_KEY);
+ String engineFull = resultSet.getString("engine_full");
+ tableProperties.put(GRAVITINO_ENGINE_KEY, engine);
+ if (StringUtils.isNotBlank(clusterName)) {
+ tableProperties.put(ClusterConstants.ON_CLUSTER,
String.valueOf(true));
+ tableProperties.put(ClusterConstants.CLUSTER_NAME, clusterName);
+ } else {
+ tableProperties.put(ClusterConstants.ON_CLUSTER,
String.valueOf(false));
+ }
+
+ if (StringUtils.equalsIgnoreCase(engine,
ENGINE.DISTRIBUTED.getValue())) {
+ Matcher distributedEngineMatcher =
+
DISTRIBUTED_ENGINE_PATTERN.matcher(StringUtils.trimToEmpty(engineFull));
+ if (distributedEngineMatcher.matches()) {
+ String distributedClusterName =
unquote(distributedEngineMatcher.group(1));
+ tableProperties.put(ClusterConstants.CLUSTER_NAME,
distributedClusterName);
+ tableProperties.put(
+ DistributedTableConstants.REMOTE_DATABASE,
+ unquote(distributedEngineMatcher.group(2)));
+ tableProperties.put(
+ DistributedTableConstants.REMOTE_TABLE,
+ unquote(distributedEngineMatcher.group(3)));
+ tableProperties.put(
+ DistributedTableConstants.SHARDING_KEY,
+ StringUtils.trim(distributedEngineMatcher.group(4)));
+ }
+ } else if (StringUtils.equalsIgnoreCase(engine,
ENGINE.GRAPHITEMERGETREE.getValue())) {
+ String graphiteConfig = extractGraphiteConfig(engineFull);
+ if (StringUtils.isNotBlank(graphiteConfig)) {
+ tableProperties.put(TableConstants.GRAPHITE_CONFIG,
graphiteConfig);
+ }
+ } else if (isGenericEngineParameterEngine(engine)) {
+ String engineParams = extractEngineParams(engine, engineFull);
+ if (StringUtils.isNotBlank(engineParams)) {
+ tableProperties.put(TableConstants.ENGINE_PARAMETERS,
engineParams);
+ }
+ }
+
+ return Collections.unmodifiableMap(tableProperties);
}
}
@@ -1642,6 +1668,144 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
.formatted(quoteIdentifier(indexName), fieldStr, typeName,
granularity);
}
+ /**
+ * Extracts engine parameters from the {@code engine_full} column of {@code
system.tables}.
+ *
+ * <p>Matches the outer parentheses while ignoring parentheses inside quoted
strings and
+ * identifiers. For example, {@code SummingMergeTree((a, b))} returns {@code
"(a, b)"}, and {@code
+ * ReplacingMergeTree(`ver)`)} returns {@code "`ver)`"}. Engines without
parameters return {@code
+ * null}.
+ */
+ @VisibleForTesting
+ @Nullable
+ static String extractEngineParams(@Nullable String engineName, @Nullable
String engineFull) {
+ if (StringUtils.isBlank(engineFull) || StringUtils.isBlank(engineName)) {
+ return null;
+ }
+
+ String normalizedEngineName = StringUtils.trim(engineName);
+ String normalizedEngineFull = StringUtils.trim(engineFull);
+ if (!StringUtils.startsWithIgnoreCase(normalizedEngineFull,
normalizedEngineName)) {
+ return null;
+ }
+
+ int paramsStart = normalizedEngineName.length();
+ while (paramsStart < normalizedEngineFull.length()
+ && Character.isWhitespace(normalizedEngineFull.charAt(paramsStart))) {
+ paramsStart++;
+ }
+ if (paramsStart >= normalizedEngineFull.length()
+ || normalizedEngineFull.charAt(paramsStart) != '(') {
+ return null;
+ }
+
+ int paramsEnd = findMatchingParenthesis(normalizedEngineFull, paramsStart);
+ if (paramsEnd < 0) {
+ return null;
+ }
+ return normalizedEngineFull.substring(paramsStart + 1, paramsEnd).trim();
+ }
+
+ private static void validateEngineParameters(ENGINE engine, @Nullable String
engineParams) {
+ if (StringUtils.isBlank(engineParams)) {
+ return;
+ }
+
+ if (engine == ENGINE.GRAPHITEMERGETREE) {
+ throw new IllegalArgumentException(
+ "'engine_parameters' is not supported for GraphiteMergeTree; use
'graphite.config'");
+ }
+ if (engine == ENGINE.DISTRIBUTED) {
+ throw new IllegalArgumentException(
+ "'engine_parameters' is not supported for Distributed; use the
distributed table "
+ + "properties");
+ }
+ Preconditions.checkArgument(
+ GENERIC_ENGINE_PARAMETER_ENGINES.contains(engine),
+ "'engine_parameters' is not supported for ClickHouse engine %s",
+ engine.getValue());
+
+ String wrappedParams = "(" + engineParams + ")";
+ Preconditions.checkArgument(
+ findMatchingParenthesis(wrappedParams, 0) == wrappedParams.length() -
1,
+ "Invalid 'engine_parameters' for ClickHouse engine %s: parentheses and
quotes must be "
+ + "balanced",
+ engine.getValue());
+ }
+
+ private static boolean isGenericEngineParameterEngine(@Nullable String
engineName) {
+ return GENERIC_ENGINE_PARAMETER_ENGINES.stream()
+ .anyMatch(engine -> StringUtils.equalsIgnoreCase(engine.getValue(),
engineName));
+ }
+
+ @Nullable
+ private static String extractGraphiteConfig(@Nullable String engineFull) {
+ String engineParams =
extractEngineParams(ENGINE.GRAPHITEMERGETREE.getValue(), engineFull);
+ if (!isSingleQuotedLiteral(engineParams)) {
+ return null;
+ }
+
+ String quotedConfig = StringUtils.trim(engineParams);
+ return JdbcConnectorUtils.unescapeSqlLiteral(
+ quotedConfig.substring(1, quotedConfig.length() - 1), '\'');
+ }
+
+ private static boolean isSingleQuotedLiteral(@Nullable String value) {
+ String literal = StringUtils.trim(value);
+ if (StringUtils.length(literal) < 2 || literal.charAt(0) != '\'') {
+ return false;
+ }
+
+ for (int i = 1; i < literal.length(); i++) {
+ char current = literal.charAt(i);
+ if (current == '\\') {
+ if (i + 1 >= literal.length()) {
+ return false;
+ }
+ i++;
+ } else if (current == '\'') {
+ if (i + 1 < literal.length() && literal.charAt(i + 1) == '\'') {
+ i++;
+ } else {
+ return i == literal.length() - 1;
+ }
+ }
+ }
+ return false;
+ }
+
+ private static int findMatchingParenthesis(String value, int
openParenthesis) {
+ int depth = 1;
+ char quote = 0;
+ for (int i = openParenthesis + 1; i < value.length(); i++) {
+ char current = value.charAt(i);
+ if (quote != 0) {
+ if (current == '\\' && i + 1 < value.length()) {
+ i++;
+ } else if (current == quote) {
+ if (i + 1 < value.length() && value.charAt(i + 1) == quote) {
+ i++;
+ } else {
+ quote = 0;
+ }
+ }
+ continue;
+ }
+
+ if (current == '\'' || current == '"' || current == '`') {
+ quote = current;
+ } else if (current == '(') {
+ depth++;
+ } else if (current == ')') {
+ depth--;
+ if (depth == 0) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
private StringBuilder appendColumnDefinition(JdbcColumn column,
StringBuilder sqlBuilder) {
// Add Nullable data type
String dataType = typeConverter.fromGravitino(column.dataType());
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 71d9bc9203..6410fc1a51 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
@@ -20,6 +20,9 @@ package
org.apache.gravitino.catalog.clickhouse.integration.test;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.MERGETREE;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.REPLACINGMERGETREE;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.SUMMINGMERGETREE;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.VERSIONEDCOLLAPSINGMERGETREE;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.GRAVITINO_ENGINE_KEY;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders;
import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
@@ -2919,6 +2922,189 @@ public class CatalogClickHouseIT extends BaseIT {
Indexes.EMPTY_INDEXES));
}
+ @Test
+ void testEngineParametersRoundTrip() {
+ // Create a ReplacingMergeTree table with engine parameter via Gravitino
API and verify
+ // engine_parameters is preserved on load (round-trip).
+ String name = GravitinoITUtils.genRandomName("engine_params_rpt");
+ NameIdentifier ident = NameIdentifier.of(schemaName, name);
+ Column[] cols =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("ts", Types.IntegerType.get(), "version", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+
+ Map<String, String> properties = createProperties();
+ properties.put(GRAVITINO_ENGINE_KEY, REPLACINGMERGETREE.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "ts");
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident,
+ cols,
+ "Engine params round-trip test",
+ properties,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertEquals(
+ "ts",
+ loadedProps.get(TableConstants.ENGINE_PARAMETERS),
+ "engine_parameters 'ts' should survive create→load round-trip");
+
+ // Verify the actual DDL in ClickHouse contains the engine parameter.
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName, name));
+ Assertions.assertTrue(
+ createSql.contains("ReplacingMergeTree(ts)"),
+ "SHOW CREATE TABLE should contain ReplacingMergeTree(ts): " +
createSql);
+ }
+
+ @Test
+ void testEngineParametersNestedParensRoundTrip() {
+ // SummingMergeTree with multi-column tuple parameter, e.g.
SummingMergeTree((a, b)).
+ String name = GravitinoITUtils.genRandomName("engine_params_nested");
+ NameIdentifier ident = NameIdentifier.of(schemaName, name);
+ Column[] cols =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("a", Types.IntegerType.get(), "a", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("b", Types.IntegerType.get(), "b", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+
+ Map<String, String> properties = createProperties();
+ properties.put(GRAVITINO_ENGINE_KEY, SUMMINGMERGETREE.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "(a, b)");
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident,
+ cols,
+ "Nested parens round-trip",
+ properties,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertEquals(
+ "(a, b)",
+ loadedProps.get(TableConstants.ENGINE_PARAMETERS),
+ "engine_parameters '(a, b)' should survive round-trip");
+
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName, name));
+ Assertions.assertTrue(
+ createSql.contains("SummingMergeTree((a, b))"),
+ "SHOW CREATE TABLE should contain SummingMergeTree((a, b)): " +
createSql);
+ }
+
+ @Test
+ void testEngineParametersMultiParamRoundTrip() {
+ // VersionedCollapsingMergeTree(sign, ver) — multi-parameter engine
round-trip.
+ String name = GravitinoITUtils.genRandomName("engine_params_multi");
+ NameIdentifier ident = NameIdentifier.of(schemaName, name);
+ Column[] cols =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("sign", Types.ByteType.get(), "sign", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("ver", Types.IntegerType.get(), "ver", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+
+ Map<String, String> properties = createProperties();
+ properties.put(GRAVITINO_ENGINE_KEY,
VERSIONEDCOLLAPSINGMERGETREE.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "sign, ver");
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident,
+ cols,
+ "Multi-param engine round-trip",
+ properties,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertEquals(
+ "sign, ver",
+ loadedProps.get(TableConstants.ENGINE_PARAMETERS),
+ "engine_parameters 'sign, ver' should survive round-trip");
+
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName, name));
+ Assertions.assertTrue(
+ createSql.contains("VersionedCollapsingMergeTree(sign, ver)"),
+ "SHOW CREATE TABLE should contain VersionedCollapsingMergeTree(sign,
ver): " + createSql);
+ }
+
+ @Test
+ void testEngineParamsLoadFromExistingTable() {
+ // Create a table directly in ClickHouse (bypass Gravitino) with engine
parameters,
+ // then load via Gravitino and verify engine_parameters is extracted.
+ String name = GravitinoITUtils.genRandomName("engine_params_load");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` (id Int32, sign Int8) "
+ + "ENGINE = CollapsingMergeTree(sign) ORDER BY id",
+ schemaName, name));
+
+ Table loaded =
catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName, name));
+ Map<String, String> props = loaded.properties();
+ Assertions.assertEquals(
+ "sign",
+ props.get(TableConstants.ENGINE_PARAMETERS),
+ "CollapsingMergeTree(sign) should have engine_parameters='sign'");
+ }
+
+ @Test
+ void testEngineParamsAbsentForParameterizedNonMergeTreeEngine() {
+ String name = GravitinoITUtils.genRandomName("engine_params_join");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` (id Int32, payload String) " + "ENGINE =
Join(ANY, LEFT, id)",
+ schemaName, name));
+
+ Table loaded =
catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName, name));
+ Assertions.assertEquals(ENGINE.JOIN.getValue(),
loaded.properties().get(GRAVITINO_ENGINE_KEY));
+ Assertions.assertFalse(
+ loaded.properties().containsKey(TableConstants.ENGINE_PARAMETERS),
+ "Parameterized non-MergeTree engines must not expose
engine_parameters");
+ }
+
+ @Test
+ void testEngineParamsAbsentForMergeTree() {
+ // MergeTree has no engine parameters — verify the property is not present.
+ String name = GravitinoITUtils.genRandomName("engine_params_none");
+ 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(GRAVITINO_ENGINE_KEY, MERGETREE.getValue());
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ ident, cols, "No engine params", properties, Distributions.NONE,
getSortOrders("id"));
+
+ Table loaded = catalog.asTableCatalog().loadTable(ident);
+ Map<String, String> loadedProps = loaded.properties();
+ Assertions.assertFalse(
+ loadedProps.containsKey(TableConstants.ENGINE_PARAMETERS),
+ "MergeTree should not have engine_parameters property");
+ }
+
@Test
void testEnumRoundTrip() {
// Create a table in ClickHouse with Enum8 and Enum16 columns
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
index e31362c782..e2cfa0ad07 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
@@ -18,15 +18,25 @@
*/
package org.apache.gravitino.catalog.clickhouse.operations;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders;
+
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.util.HashMap;
import java.util.List;
+import java.util.Map;
+import
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.TableConstants;
+import
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseColumnDefaultValueConverter;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseExceptionConverter;
import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter;
+import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.transforms.Transforms;
import org.apache.gravitino.rel.indexes.Index;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.rel.types.Types;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
@@ -39,6 +49,31 @@ public class TestClickHouseTableOperationsUnit {
throws Exception {
return getIndexes(connection, databaseName, tableName);
}
+
+ Map<String, String> callGetTableProperties(Connection connection, String
tableName)
+ throws Exception {
+ return getTableProperties(connection, tableName);
+ }
+
+ String callGenerateCreateTableSql(Map<String, String> properties) {
+ JdbcColumn[] columns =
+ new JdbcColumn[] {
+ JdbcColumn.builder()
+ .withName("id")
+ .withType(Types.IntegerType.get())
+ .withNullable(false)
+ .build()
+ };
+ return generateCreateTableSql(
+ "test_table",
+ columns,
+ "",
+ properties,
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES,
+ getSortOrders("id"));
+ }
}
private ExposedClickHouseTableOperations newOps() {
@@ -52,6 +87,22 @@ public class TestClickHouseTableOperationsUnit {
return ops;
}
+ private Map<String, String> loadTableProperties(String engine, String
engineFull)
+ throws Exception {
+ PreparedStatement statement = Mockito.mock(PreparedStatement.class);
+ ResultSet resultSet = Mockito.mock(ResultSet.class);
+ Mockito.when(resultSet.next()).thenReturn(true);
+ Mockito.when(resultSet.getString("name")).thenReturn("test_table");
+ Mockito.when(resultSet.getString("COMMENT")).thenReturn("");
+ Mockito.when(resultSet.getString("ENGINE")).thenReturn(engine);
+ Mockito.when(resultSet.getString("engine_full")).thenReturn(engineFull);
+ Mockito.when(statement.executeQuery()).thenReturn(resultSet);
+
+ Connection connection = Mockito.mock(Connection.class);
+
Mockito.when(connection.prepareStatement(Mockito.anyString())).thenReturn(statement);
+ return newOps().callGetTableProperties(connection, "test_table");
+ }
+
//
---------------------------------------------------------------------------
// getIndexes — SQL injection escape
//
---------------------------------------------------------------------------
@@ -84,4 +135,217 @@ public class TestClickHouseTableOperationsUnit {
primaryKeySql.contains("db''1"), "database single quote should be
doubled");
Assertions.assertTrue(primaryKeySql.contains("t''1"), "table single quote
should be doubled");
}
+
+ //
---------------------------------------------------------------------------
+ // extractEngineParams
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void testExtractEngineParamsWithParams() {
+ Assertions.assertEquals(
+ "ts",
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree",
+ "ReplacingMergeTree(ts) ORDER BY id SETTINGS index_granularity =
8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsMultipleParams() {
+ Assertions.assertEquals(
+ "sign, ts",
+ ClickHouseTableOperations.extractEngineParams(
+ "VersionedCollapsingMergeTree",
+ "VersionedCollapsingMergeTree(sign, ts) ORDER BY id SETTINGS
index_granularity = 8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsSingleParam() {
+ Assertions.assertEquals(
+ "sign",
+ ClickHouseTableOperations.extractEngineParams(
+ "CollapsingMergeTree",
+ "CollapsingMergeTree(sign) ORDER BY id SETTINGS index_granularity
= 8192"));
+ Assertions.assertEquals(
+ "val",
+ ClickHouseTableOperations.extractEngineParams(
+ "SummingMergeTree",
+ "SummingMergeTree(val) ORDER BY id SETTINGS index_granularity =
8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsNoParams() {
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams(
+ "MergeTree", "MergeTree ORDER BY id SETTINGS index_granularity =
8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsBlankInput() {
+
Assertions.assertNull(ClickHouseTableOperations.extractEngineParams("MergeTree",
null));
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams(null, "MergeTree ORDER
BY id"));
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams("", "MergeTree ORDER BY
id"));
+ }
+
+ @Test
+ void testExtractEngineParamsEngineNameNotAtStart() {
+ // The engine name must be at the start of engine_full.
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams("MergeTree", "something
else MergeTree(x)"));
+ }
+
+ @Test
+ void testExtractEngineParamsNestedParens() {
+ // SummingMergeTree((a, b)) — nested parentheses should be preserved.
+ Assertions.assertEquals(
+ "(a, b)",
+ ClickHouseTableOperations.extractEngineParams(
+ "SummingMergeTree",
+ "SummingMergeTree((a, b)) ORDER BY id SETTINGS index_granularity =
8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsGraphiteMergeTree() {
+ // The generic scanner preserves the quoted parameter for
Graphite-specific decoding.
+ Assertions.assertEquals(
+ "'graphite_rollup'",
+ ClickHouseTableOperations.extractEngineParams(
+ "GraphiteMergeTree",
+ "GraphiteMergeTree('graphite_rollup') ORDER BY id SETTINGS
index_granularity = 8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsAggregatingMergeTree() {
+ // AggregatingMergeTree has no parameters.
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams(
+ "AggregatingMergeTree",
+ "AggregatingMergeTree ORDER BY id SETTINGS index_granularity =
8192"));
+ }
+
+ @Test
+ void testExtractEngineParamsIgnoresParenthesesInsideQuotes() {
+ Assertions.assertEquals(
+ "`ver)`",
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", "ReplacingMergeTree(`ver)`) ORDER BY id"));
+ Assertions.assertEquals(
+ "\"ver)\"",
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", "ReplacingMergeTree(\"ver)\") ORDER BY id"));
+ Assertions.assertEquals(
+ "'rollup(test)'",
+ ClickHouseTableOperations.extractEngineParams(
+ "GraphiteMergeTree", "GraphiteMergeTree('rollup(test)') ORDER BY
id"));
+ }
+
+ @Test
+ void testExtractEngineParamsHandlesEscapedQuotes() {
+ Assertions.assertEquals(
+ "`ver``)`",
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", "ReplacingMergeTree(`ver``)`) ORDER BY id"));
+ Assertions.assertEquals(
+ "'rollup\\')test'",
+ ClickHouseTableOperations.extractEngineParams(
+ "GraphiteMergeTree", "GraphiteMergeTree('rollup\\')test') ORDER BY
id"));
+ }
+
+ @Test
+ void testExtractEngineParamsAllowsWhitespaceBeforeParameters() {
+ Assertions.assertEquals(
+ "ts",
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", " ReplacingMergeTree \t (ts) ORDER BY id"));
+ }
+
+ @Test
+ void testExtractEngineParamsRejectsUnclosedInput() {
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", "ReplacingMergeTree('ver)) ORDER BY id"));
+ Assertions.assertNull(
+ ClickHouseTableOperations.extractEngineParams(
+ "ReplacingMergeTree", "ReplacingMergeTree(tuple(ts) ORDER BY id"));
+ }
+
+ @Test
+ void testGraphitePropertiesRoundTripThroughSqlGeneration() throws Exception {
+ Map<String, String> loadedProperties =
+ loadTableProperties(
+ ENGINE.GRAPHITEMERGETREE.getValue(),
+ "GraphiteMergeTree('graphite''rollup\\\\path') ORDER BY id");
+
+ Assertions.assertEquals(
+ "graphite'rollup\\path",
loadedProperties.get(TableConstants.GRAPHITE_CONFIG));
+
Assertions.assertFalse(loadedProperties.containsKey(TableConstants.ENGINE_PARAMETERS));
+
+ String createSql = newOps().callGenerateCreateTableSql(loadedProperties);
+ Assertions.assertTrue(
+ createSql.contains("ENGINE =
GraphiteMergeTree('graphite''rollup\\\\path')"), createSql);
+ }
+
+ @Test
+ void testNonMergeTreeLoadDoesNotExposeEngineParameters() throws Exception {
+ Map<String, String> loadedProperties =
+ loadTableProperties(
+ ENGINE.MySQL.getValue(), "MySQL('host:9000', 'database', 'table',
'user', 'secret')");
+
+
Assertions.assertFalse(loadedProperties.containsKey(TableConstants.ENGINE_PARAMETERS));
+ Assertions.assertFalse(
+ loadedProperties.values().stream()
+ .anyMatch(value -> value != null && value.contains("secret")));
+ }
+
+ @Test
+ void testSupportedEngineParametersGenerateSql() {
+ Map<String, String> properties = new HashMap<>();
+ properties.put("engine", ENGINE.SUMMINGMERGETREE.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "(id)");
+
+ String createSql = newOps().callGenerateCreateTableSql(properties);
+ Assertions.assertTrue(createSql.contains("ENGINE =
SummingMergeTree((id))"), createSql);
+ }
+
+ @Test
+ void testUnsupportedEnginesRejectGenericParameters() {
+ for (ENGINE engine :
+ List.of(
+ ENGINE.MERGETREE,
+ ENGINE.AGGREGATINGMERGETREE,
+ ENGINE.JOIN,
+ ENGINE.MySQL,
+ ENGINE.GRAPHITEMERGETREE,
+ ENGINE.DISTRIBUTED)) {
+ Map<String, String> properties = new HashMap<>();
+ properties.put("engine", engine.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "sensitive_value");
+ if (engine == ENGINE.GRAPHITEMERGETREE) {
+ properties.put(TableConstants.GRAPHITE_CONFIG, "graphite_rollup");
+ }
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> newOps().callGenerateCreateTableSql(properties),
+ engine.getValue());
+
Assertions.assertTrue(exception.getMessage().contains("engine_parameters"));
+ if (engine == ENGINE.GRAPHITEMERGETREE) {
+
Assertions.assertTrue(exception.getMessage().contains("graphite.config"));
+ }
+ }
+ }
+
+ @Test
+ void testSupportedEngineParametersCannotEscapeEngineClause() {
+ Map<String, String> properties = new HashMap<>();
+ properties.put("engine", ENGINE.REPLACINGMERGETREE.getValue());
+ properties.put(TableConstants.ENGINE_PARAMETERS, "ts) SETTINGS
index_granularity = 1");
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
newOps().callGenerateCreateTableSql(properties));
+ Assertions.assertTrue(exception.getMessage().contains("balanced"));
+ }
}
