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 93e487707c [#12787] fix(clickhouse): preserve SET index parameters on
table load (#12826)
93e487707c is described below
commit 93e487707ccf86e16e6dff2dc78c8673ea4cc5fa
Author: StormSpirit <[email protected]>
AuthorDate: Mon Sep 21 15:07:55 2026 +0800
[#12787] fix(clickhouse): preserve SET index parameters on table load
(#12826)
### What changes were proposed in this pull request?
This pull request restores the `set_max_values` property when loading
ClickHouse `DATA_SKIPPING_SET` indexes from
`system.data_skipping_indices.type_full`.
The implementation adds a narrow SET parameter parser with overflow-safe
validation, preserves the default `set(0)` behavior, supports the modern
`type_full` path and legacy parameterized fallback, and keeps malformed
or out-of-range metadata from being silently degraded. Existing
ngram/token expression-skip behavior is preserved.
The tests cover modern and legacy metadata, default values, malformed
metadata, `Integer.MAX_VALUE`, `Integer.MAX_VALUE + 1`, the unsigned
64-bit maximum, granularity coexistence, and a real
create/load/alter/adoption/recreate lifecycle.
### Why are the changes needed?
ClickHouse preserves a SET index parameter such as `set(100)` in its
metadata, but the ClickHouse catalog previously loaded only the index
type and granularity. The missing parameter caused a later table
recreation to emit `set(0)`, changing the index configuration and
potentially its data-skipping behavior.
Fix: #12787
### Does this PR introduce _any_ user-facing change?
Loaded SET indexes with `set_max_values` in the supported range
`0..Integer.MAX_VALUE` now expose the canonical `set_max_values`
property. The default value `0` remains omitted from
`Index.properties()`. Malformed SET metadata now fails with an
index-specific error identifying the metadata source. Out-of-range
metadata additionally reports the supported range.
No public API, OpenAPI field, or ClickHouse server-version support claim
is changed.
### How was this patch tested?
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:spotlessCheck` —
passed.
- `./gradlew rat` — passed.
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs` —
94 tests passed, 0 failures, 0 errors, 0 skipped.
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests
'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT.testSetIndexParameterReadbackLifecycle'
-PskipDockerTests=false --console=plain --no-daemon` on ClickHouse
`24.8.14` — 1 test passed.
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests
'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseClusterIT'
-PskipDockerTests=false --console=plain --no-daemon` on ClickHouse
`24.8.14` — 18 tests passed.
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build -x test` —
passed.
- Supplemental local Gravitino precheck — passed with no errors.
The focused and cluster Docker tests passed before the final
diagnostics-only review follow-up; the follow-up changed only SET error
wording and mocked query-path coverage, without changing
CREATE/ALTER/readback lifecycle behavior, so the long IT suites were not
rerun.
---------
Signed-off-by: jiangxt2 <[email protected]>
---
.../operations/ClickHouseTableOperations.java | 157 +++++++++++-
.../integration/test/CatalogClickHouseIT.java | 117 +++++++++
.../TestClickHouseTableOperationsUnit.java | 284 ++++++++++++++++++++-
3 files changed, 538 insertions(+), 20 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 21f824a9fe..e2236b4003 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
@@ -37,6 +37,7 @@ import static
org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import java.math.BigInteger;
import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.PreparedStatement;
@@ -105,6 +106,12 @@ 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 BigInteger MIN_SET_MAX_VALUES = BigInteger.ZERO;
+ private static final BigInteger MAX_SET_MAX_VALUES =
BigInteger.valueOf(Integer.MAX_VALUE);
+ private static final String SET_MAX_VALUES_RANGE =
+ "[%s, %s]".formatted(MIN_SET_MAX_VALUES, MAX_SET_MAX_VALUES);
+ private static final Pattern SET_MAX_VALUES_PATTERN =
Pattern.compile("[+-]?[0-9]+");
+
private static final Set<ENGINE> GENERIC_ENGINE_PARAMETER_ENGINES =
Collections.unmodifiableSet(
EnumSet.of(
@@ -1895,7 +1902,6 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
String expression = resultSet.getString("expr");
long granularity = resultSet.getLong("granularity");
Index.IndexType indexType;
- String[][] fields;
try {
indexType = getClickHouseIndexType(type);
} catch (IllegalArgumentException ignored) {
@@ -1907,6 +1913,9 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
type);
continue;
}
+
+ Map<String, String> parameterProperties = Collections.emptyMap();
+ String[][] fields;
try {
fields = parseIndexFields(expression);
} catch (IllegalArgumentException ignored) {
@@ -1923,6 +1932,25 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
continue;
}
+ if (indexType == Index.IndexType.DATA_SKIPPING_SET
+ || isParameterizedBloomFilterIndex(indexType)) {
+ try {
+ parameterProperties =
+ parseIndexPropertiesForQuery(indexType, parameterSource,
name, !includesTypeFull);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException(
+ "Failed to load data skipping index '%s' from %s.%s with %s
'%s': %s"
+ .formatted(
+ name,
+ databaseName,
+ tableName,
+ parameterSourceName,
+ parameterSource,
+ e.getMessage()),
+ e);
+ }
+ }
+
// Only include granularity in properties when it differs from the
default,
// so that indexes created without explicit granularity have empty
properties
// and match the original creation state (avoids false index-change
diffs).
@@ -1930,20 +1958,9 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
if (granularity != DEFAULT_INDEX_GRANULARITY) {
properties.put(GRANULARITY, String.valueOf(granularity));
}
- Map<String, String> bloomFilterProperties;
- try {
- bloomFilterProperties =
- parseBloomFilterPropertiesForQuery(
- indexType, parameterSource, name, !includesTypeFull);
- } catch (IllegalArgumentException e) {
- throw new IllegalArgumentException(
- "Failed to load data skipping index '%s' from %s.%s with %s
'%s'"
- .formatted(name, databaseName, tableName,
parameterSourceName, parameterSource),
- e);
- }
if (!includesTypeFull
&& isParameterizedBloomFilterIndex(indexType)
- && bloomFilterProperties.isEmpty()) {
+ && parameterProperties.isEmpty()) {
LOG.warn(
"Legacy ClickHouse metadata does not expose bloom-filter
parameters for "
+ "{} index '{}' on {}.{}; loaded Index.properties() is
incomplete",
@@ -1952,7 +1969,18 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
databaseName,
tableName);
}
- properties.putAll(bloomFilterProperties);
+ if (!includesTypeFull
+ && indexType == Index.IndexType.DATA_SKIPPING_SET
+ && parameterProperties.isEmpty()
+ && !StringUtils.contains(parameterSource, "(")) {
+ LOG.warn(
+ "Legacy ClickHouse metadata does not expose SET max-values
parameters for "
+ + "SET index '{}' on {}.{}; loaded Index.properties() is
incomplete",
+ name,
+ databaseName,
+ tableName);
+ }
+ properties.putAll(parameterProperties);
secondaryIndexes.add(Indexes.of(indexType, name, fields,
properties));
}
}
@@ -1977,6 +2005,107 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
return false;
}
+ /**
+ * Parses the single positional parameter returned by ClickHouse for a SET
data skipping index.
+ *
+ * @param indexType the mapped Gravitino index type
+ * @param typeFull the complete ClickHouse index type expression
+ * @param indexName the index name for validation messages
+ * @return the SET index properties, or an empty map for non-SET index types
and {@code set(0)}
+ * @throws IllegalArgumentException if a SET index has malformed or
out-of-range parameters
+ */
+ @VisibleForTesting
+ static Map<String, String> parseSetProperties(
+ Index.IndexType indexType, String typeFull, String indexName) {
+ if (indexType != Index.IndexType.DATA_SKIPPING_SET) {
+ return Collections.emptyMap();
+ }
+
+ String normalizedTypeFull = StringUtils.trimToEmpty(typeFull);
+ int paramsStart = normalizedTypeFull.indexOf('(');
+ int paramsEnd = normalizedTypeFull.lastIndexOf(')');
+ Preconditions.checkArgument(
+ paramsStart > 0 && paramsEnd == normalizedTypeFull.length() - 1,
+ "Invalid SET metadata '%s' for index '%s'",
+ typeFull,
+ indexName);
+ Preconditions.checkArgument(
+ StringUtils.equalsIgnoreCase(
+ DATA_SKIPPING_SET, normalizedTypeFull.substring(0,
paramsStart).trim()),
+ "SET metadata '%s' does not match SET index '%s'",
+ typeFull,
+ indexName);
+
+ String[] params = normalizedTypeFull.substring(paramsStart + 1,
paramsEnd).split(",", -1);
+ Preconditions.checkArgument(
+ params.length == 1,
+ "Invalid SET metadata '%s' for SET index '%s': expected one parameter
but got %s",
+ typeFull,
+ indexName,
+ params.length);
+
+ String rawValue = params[0].trim();
+ Preconditions.checkArgument(
+ !rawValue.isEmpty(),
+ "Invalid SET metadata '%s' for SET index '%s': set_max_values is
required",
+ typeFull,
+ indexName);
+ Preconditions.checkArgument(
+ SET_MAX_VALUES_PATTERN.matcher(rawValue).matches(),
+ "Invalid SET metadata '%s' for SET index '%s': set_max_values '%s' is
not a valid decimal integer",
+ typeFull,
+ indexName,
+ rawValue);
+ BigInteger value;
+ try {
+ value = new BigInteger(rawValue);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Invalid SET metadata '%s' for SET index '%s': set_max_values '%s'
is not a valid decimal integer"
+ .formatted(typeFull, indexName, rawValue),
+ e);
+ }
+
+ if (value.compareTo(MIN_SET_MAX_VALUES) < 0 ||
value.compareTo(MAX_SET_MAX_VALUES) > 0) {
+ throw new IllegalArgumentException(
+ "Invalid SET metadata '%s' for SET index '%s': set_max_values '%s'
is outside supported range %s"
+ .formatted(typeFull, indexName, rawValue, SET_MAX_VALUES_RANGE));
+ }
+ if (value.equals(MIN_SET_MAX_VALUES)) {
+ return Collections.emptyMap();
+ }
+ return Map.of(SET_MAX_VALUES, value.toString());
+ }
+
+ private static Map<String, String> parseIndexPropertiesForQuery(
+ Index.IndexType indexType,
+ String parameterSource,
+ String indexName,
+ boolean allowBareLegacyType) {
+ switch (indexType) {
+ case DATA_SKIPPING_SET:
+ return parseSetPropertiesForQuery(
+ indexType, parameterSource, indexName, allowBareLegacyType);
+ case DATA_SKIPPING_NGRAMBFV1:
+ case DATA_SKIPPING_TOKENBFV1:
+ return parseBloomFilterPropertiesForQuery(
+ indexType, parameterSource, indexName, allowBareLegacyType);
+ default:
+ return Collections.emptyMap();
+ }
+ }
+
+ private static Map<String, String> parseSetPropertiesForQuery(
+ Index.IndexType indexType,
+ String parameterSource,
+ String indexName,
+ boolean allowBareLegacyType) {
+ if (allowBareLegacyType && !StringUtils.contains(parameterSource, "(")) {
+ return Collections.emptyMap();
+ }
+ return parseSetProperties(indexType, parameterSource, indexName);
+ }
+
/**
* Parses the positional parameters returned by ClickHouse in {@code
type_full} for the two
* parameterized bloom-filter data skipping indexes.
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 769fd71906..9e7dba6e81 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
@@ -18,6 +18,8 @@
*/
package org.apache.gravitino.catalog.clickhouse.integration.test;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.IndexConstants.GRANULARITY;
+import static
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.IndexConstants.SET_MAX_VALUES;
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;
@@ -766,6 +768,121 @@ public class CatalogClickHouseIT extends BaseIT {
&& Arrays.deepEquals(idx.fieldNames(), new String[][]
{{"user_id"}})));
}
+ @Test
+ void testSetIndexParameterReadbackLifecycle() {
+ TableCatalog tableCatalog = catalog.asTableCatalog();
+ Column[] columns =
+ new Column[] {
+ Column.of("id", Types.LongType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("value", Types.StringType.get(), "value", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+ SortOrder[] sortOrders = getSortOrders("id");
+ Map<String, String> setProperties = Map.of(SET_MAX_VALUES, "100",
GRANULARITY, "3");
+
+ String tableName = GravitinoITUtils.genRandomName("set_readback");
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
+ tableCatalog.createTable(
+ tableIdentifier,
+ columns,
+ "SET index readback",
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ sortOrders,
+ new Index[] {
+ Indexes.of(
+ Index.IndexType.DATA_SKIPPING_SET,
+ "idx_set",
+ new String[][] {{"value"}},
+ setProperties)
+ });
+
+ Table loaded = tableCatalog.loadTable(tableIdentifier);
+ assertSetIndexMetadata(loaded, "idx_set", setProperties);
+ assertSetIndexDdl(tableName, "set(100)", "GRANULARITY 3");
+ Index[] loadedIndexes = loaded.index();
+
+ String recreatedTableName =
GravitinoITUtils.genRandomName("set_recreated");
+ NameIdentifier recreatedIdentifier = NameIdentifier.of(schemaName,
recreatedTableName);
+ tableCatalog.createTable(
+ recreatedIdentifier,
+ columns,
+ "SET index recreated",
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ sortOrders,
+ loadedIndexes);
+ Table recreated = tableCatalog.loadTable(recreatedIdentifier);
+ assertSetIndexMetadata(recreated, "idx_set", setProperties);
+ assertSetIndexDdl(recreatedTableName, "set(100)", "GRANULARITY 3");
+
+ tableCatalog.alterTable(
+ tableIdentifier,
+ TableChange.addIndex(
+ Index.IndexType.DATA_SKIPPING_SET,
+ "idx_set_alter",
+ new String[][] {{"value"}},
+ setProperties));
+ Table altered = tableCatalog.loadTable(tableIdentifier);
+ assertSetIndexMetadata(altered, "idx_set_alter", setProperties);
+ assertSetIndexDdl(tableName, "set(100)", "GRANULARITY 3");
+
+ String defaultTableName = GravitinoITUtils.genRandomName("set_default");
+ NameIdentifier defaultIdentifier = NameIdentifier.of(schemaName,
defaultTableName);
+ tableCatalog.createTable(
+ defaultIdentifier,
+ columns,
+ "SET index default",
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ sortOrders,
+ new Index[] {
+ Indexes.of(
+ Index.IndexType.DATA_SKIPPING_SET, "idx_set_default", new
String[][] {{"value"}})
+ });
+ Table defaultLoaded = tableCatalog.loadTable(defaultIdentifier);
+ assertSetIndexMetadata(defaultLoaded, "idx_set_default", Map.of());
+ assertSetIndexDdl(defaultTableName, "set(0)");
+
+ String nativeTableName = GravitinoITUtils.genRandomName("set_native");
+ clickhouseService.executeQuery(
+ String.format(
+ "CREATE TABLE `%s`.`%s` ("
+ + " `id` UInt64,"
+ + " `value` String,"
+ + " INDEX `idx_native_set` `value` TYPE set(100) GRANULARITY
3"
+ + ") ENGINE = MergeTree ORDER BY id",
+ schemaName, nativeTableName));
+ Table nativeLoaded = tableCatalog.loadTable(NameIdentifier.of(schemaName,
nativeTableName));
+ assertSetIndexMetadata(nativeLoaded, "idx_native_set", setProperties);
+ }
+
+ private void assertSetIndexMetadata(
+ Table table, String indexName, Map<String, String> expectedProperties) {
+ Index index =
+ Arrays.stream(table.index())
+ .filter(candidate -> Objects.equals(indexName, candidate.name()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Missing index " +
indexName));
+ Assertions.assertEquals(Index.IndexType.DATA_SKIPPING_SET, index.type());
+ Assertions.assertArrayEquals(new String[][] {{"value"}},
index.fieldNames());
+ Assertions.assertEquals(expectedProperties, index.properties());
+ }
+
+ private void assertSetIndexDdl(String tableName, String...
expectedFragments) {
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName,
tableName));
+ String normalizedCreateSql = createSql.replaceAll("\\s+", "");
+ for (String expectedFragment : expectedFragments) {
+ Assertions.assertTrue(
+ normalizedCreateSql.contains(expectedFragment.replaceAll("\\s+",
"")),
+ "SHOW CREATE TABLE should contain " + expectedFragment + ": " +
createSql);
+ }
+ }
+
@Test
void testCreateAndLoadWithCustomGranularity() {
String table = GravitinoITUtils.genRandomName("granularity_roundtrip");
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 e5688697bc..0616db0516 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
@@ -917,6 +917,110 @@ public class TestClickHouseTableOperationsUnit {
Assertions.assertTrue(exception.getMessage().contains("type_full"));
}
+ @Test
+ void testParseSetPropertiesNormalizesValuesAndOmitsDefault() {
+ Assertions.assertEquals(
+ Map.of("set_max_values", "100"),
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, " set ( 00100 ) ", "idx_set"));
+ Assertions.assertTrue(
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, "set(0)", "idx_set")
+ .isEmpty());
+ Assertions.assertTrue(
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_MINMAX, "minmax", "idx_minmax")
+ .isEmpty());
+ }
+
+ @Test
+ void testParseSetPropertiesAcceptsIntegerMaxValue() {
+ Assertions.assertEquals(
+ Map.of("set_max_values", String.valueOf(Integer.MAX_VALUE)),
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, "set(" + Integer.MAX_VALUE +
")", "idx_set"));
+ }
+
+ @Test
+ void testParseSetPropertiesRejectsValuesOutsideIntegerRange() {
+ for (String value : List.of("-1", "2147483648", "18446744073709551615")) {
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, "set(" + value + ")",
"idx_set"));
+ Assertions.assertTrue(exception.getMessage().contains("outside supported
range"));
+ Assertions.assertTrue(exception.getMessage().contains(value));
+ Assertions.assertTrue(exception.getMessage().contains("[0,
2147483647]"));
+ Assertions.assertTrue(exception.getMessage().contains("idx_set"));
+ }
+ }
+
+ @Test
+ void testParseSetPropertiesRejectsMalformedMetadata() {
+ for (String typeFull : List.of("set()", "set(100, 200)", "set(abc)",
"set(100")) {
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, typeFull, "idx_bad"));
+ Assertions.assertTrue(exception.getMessage().contains("idx_bad"));
+ }
+
+ IllegalArgumentException wrongTypeException =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ClickHouseTableOperations.parseSetProperties(
+ Index.IndexType.DATA_SKIPPING_SET, "tokenbf_v1(100)",
"idx_bad"));
+ Assertions.assertTrue(wrongTypeException.getMessage().contains("idx_bad"));
+ }
+
+ @Test
+ void testGetIndexesFailsOnOutOfRangeSetMetadata() throws Exception {
+ for (String value : List.of("2147483648", "18446744073709551615")) {
+ IllegalArgumentException exception =
getIndexesFailureForSetTypeFull("set(" + value + ")");
+ Assertions.assertTrue(exception.getMessage().contains("idx_overflow"));
+ Assertions.assertTrue(exception.getMessage().contains("type_full"));
+ Assertions.assertTrue(exception.getMessage().contains(value));
+ Assertions.assertTrue(exception.getMessage().contains("outside supported
range"));
+ Assertions.assertTrue(exception.getMessage().contains("[0,
2147483647]"));
+ }
+ }
+
+ @Test
+ void testGetIndexesReadsSetPropertiesWithGranularity() throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement secondaryStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet secondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(secondaryRs.next()).thenReturn(true, false);
+ Mockito.when(secondaryStmt.executeQuery()).thenReturn(secondaryRs);
+ Mockito.when(secondaryRs.getString("name")).thenReturn("idx_set");
+ Mockito.when(secondaryRs.getString("type")).thenReturn("set");
+ Mockito.when(secondaryRs.getString("type_full")).thenReturn("set(100)");
+ Mockito.when(secondaryRs.getString("expr")).thenReturn("col_1");
+ Mockito.when(secondaryRs.getLong("granularity")).thenReturn(3L);
+
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(connection.prepareStatement(Mockito.anyString()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(secondaryStmt);
+
+ List<Index> indexes = ops.callGetIndexes(connection, "db", "tbl");
+
+ Assertions.assertEquals(1, indexes.size());
+ Assertions.assertEquals(
+ Map.of("set_max_values", "100", "granularity", "3"),
indexes.get(0).properties());
+ }
+
@Test
void testGetIndexesSkipsUnsupportedExpressionForParameterizedIndex() throws
Exception {
ExposedClickHouseTableOperations ops = newOps();
@@ -928,14 +1032,16 @@ public class TestClickHouseTableOperationsUnit {
Mockito.when(primaryKeyRs.next()).thenReturn(false);
Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
- Mockito.when(secondaryRs.next()).thenReturn(true, true, false);
+ Mockito.when(secondaryRs.next()).thenReturn(true, true, true, false);
Mockito.when(secondaryStmt.executeQuery()).thenReturn(secondaryRs);
- Mockito.when(secondaryRs.getString("name")).thenReturn("idx_bad_expr",
"idx_valid");
- Mockito.when(secondaryRs.getString("type")).thenReturn("ngrambf_v1",
"tokenbf_v1");
+ Mockito.when(secondaryRs.getString("name"))
+ .thenReturn("idx_bad_expr", "idx_valid", "idx_set_bad_expr");
+ Mockito.when(secondaryRs.getString("type")).thenReturn("ngrambf_v1",
"tokenbf_v1", "set");
Mockito.when(secondaryRs.getString("type_full"))
- .thenReturn("ngrambf_v1(3, 512, 3, 0)", "tokenbf_v1(256, 2, 0)");
- Mockito.when(secondaryRs.getString("expr")).thenReturn("lower(col_1)",
"col_2");
- Mockito.when(secondaryRs.getLong("granularity")).thenReturn(1L, 1L);
+ .thenReturn("ngrambf_v1(3, 512, 3, 0)", "tokenbf_v1(256, 2, 0)",
"set(100)");
+ Mockito.when(secondaryRs.getString("expr"))
+ .thenReturn("lower(col_1)", "col_2", "cityHash64(col_3) % 16");
+ Mockito.when(secondaryRs.getLong("granularity")).thenReturn(1L, 1L, 1L);
Connection connection = Mockito.mock(Connection.class);
Mockito.when(connection.prepareStatement(Mockito.anyString()))
@@ -954,6 +1060,63 @@ public class TestClickHouseTableOperationsUnit {
"hash_functions", "2",
"random_seed", "0"),
indexes.get(0).properties());
+ Assertions.assertFalse(
+ indexes.stream().anyMatch(index ->
"idx_set_bad_expr".equals(index.name())));
+ }
+
+ @Test
+ void testGetIndexesFailsOnMalformedSetMetadataWithSupportedExpression()
throws Exception {
+ IllegalArgumentException exception =
getIndexesFailureForSetMetadata("set(abc)", "col_1");
+
+
Assertions.assertTrue(exception.getMessage().contains("idx_bad_set_metadata"));
+ Assertions.assertTrue(exception.getMessage().contains("type_full"));
+ Assertions.assertTrue(exception.getMessage().contains("set(abc)"));
+ Assertions.assertTrue(exception.getMessage().contains("SET metadata"));
+ }
+
+ @Test
+ void
testGetIndexesSkipsUnsupportedExpressionBeforeParsingOutOfRangeSetMetadata()
+ throws Exception {
+ List<Index> indexes = getIndexesForSetMetadata("set(2147483648)",
"cityHash64(col_1) % 16");
+
+ Assertions.assertTrue(indexes.isEmpty());
+ }
+
+ @Test
+ void testGetIndexesFailsOnMalformedLegacySetMetadata() throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement modernSecondaryStmt =
Mockito.mock(PreparedStatement.class);
+ PreparedStatement legacySecondaryStmt =
Mockito.mock(PreparedStatement.class);
+ ResultSet legacySecondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(modernSecondaryStmt.executeQuery())
+ .thenThrow(new SQLException("Unknown identifier 'type_full'"));
+
Mockito.when(legacySecondaryStmt.executeQuery()).thenReturn(legacySecondaryRs);
+ Mockito.when(legacySecondaryRs.next()).thenReturn(true, false);
+
Mockito.when(legacySecondaryRs.getString("name")).thenReturn("idx_legacy_bad");
+ Mockito.when(legacySecondaryRs.getString("type")).thenReturn("set(abc)");
+ Mockito.when(legacySecondaryRs.getString("expr")).thenReturn("col_1");
+ Mockito.when(legacySecondaryRs.getLong("granularity")).thenReturn(1L);
+
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(connection.prepareStatement(Mockito.anyString()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(modernSecondaryStmt)
+ .thenReturn(legacySecondaryStmt);
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
ops.callGetIndexes(connection, "db", "tbl"));
+ Assertions.assertTrue(exception.getMessage().contains("idx_legacy_bad"));
+ Assertions.assertTrue(exception.getMessage().contains("legacy type"));
+ Assertions.assertTrue(exception.getMessage().contains("set(abc)"));
+ Assertions.assertTrue(exception.getMessage().contains("SET metadata"));
+ Assertions.assertFalse(exception.getMessage().contains("type_full"));
}
@Test
@@ -996,6 +1159,56 @@ public class TestClickHouseTableOperationsUnit {
indexes.get(0).properties());
}
+ @Test
+ void testGetIndexesFallsBackAndReadsLegacySetParameters() throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement modernSecondaryStmt =
Mockito.mock(PreparedStatement.class);
+ PreparedStatement legacySecondaryStmt =
Mockito.mock(PreparedStatement.class);
+ ResultSet legacySecondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(modernSecondaryStmt.executeQuery())
+ .thenThrow(new SQLException("Unknown identifier 'type_full'"));
+
Mockito.when(legacySecondaryStmt.executeQuery()).thenReturn(legacySecondaryRs);
+ Mockito.when(legacySecondaryRs.next()).thenReturn(true, true, false);
+ Mockito.when(legacySecondaryRs.getString("name"))
+ .thenReturn("idx_legacy_set", "idx_legacy_bare");
+ Mockito.when(legacySecondaryRs.getString("type")).thenReturn("set(100)",
"set");
+ Mockito.when(legacySecondaryRs.getString("expr")).thenReturn("col_1",
"col_2");
+ Mockito.when(legacySecondaryRs.getLong("granularity")).thenReturn(1L, 1L);
+
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(connection.prepareStatement(Mockito.anyString()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(modernSecondaryStmt)
+ .thenReturn(legacySecondaryStmt);
+
+ List<Index> indexes = ops.callGetIndexes(connection, "db", "tbl");
+
+ Assertions.assertEquals(2, indexes.size());
+ Index parameterized =
+ indexes.stream()
+ .filter(index -> "idx_legacy_set".equals(index.name()))
+ .findFirst()
+ .orElseThrow();
+ Assertions.assertEquals("idx_legacy_set", parameterized.name());
+ Assertions.assertEquals(Index.IndexType.DATA_SKIPPING_SET,
parameterized.type());
+ Assertions.assertEquals(Map.of("set_max_values", "100"),
parameterized.properties());
+
+ Index bare =
+ indexes.stream()
+ .filter(index -> "idx_legacy_bare".equals(index.name()))
+ .findFirst()
+ .orElseThrow();
+ Assertions.assertEquals("idx_legacy_bare", bare.name());
+ Assertions.assertEquals(Index.IndexType.DATA_SKIPPING_SET, bare.type());
+ Assertions.assertTrue(bare.properties().isEmpty());
+ }
+
@Test
void testGetIndexesDoesNotFallbackForOtherSqlErrors() throws Exception {
ExposedClickHouseTableOperations ops = newOps();
@@ -1232,4 +1445,63 @@ public class TestClickHouseTableOperationsUnit {
this.updateStatement = updateStatement;
}
}
+
+ private IllegalArgumentException getIndexesFailureForSetTypeFull(String
typeFull)
+ throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement secondaryStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet secondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(secondaryRs.next()).thenReturn(true, false);
+ Mockito.when(secondaryStmt.executeQuery()).thenReturn(secondaryRs);
+ Mockito.when(secondaryRs.getString("name")).thenReturn("idx_overflow");
+ Mockito.when(secondaryRs.getString("type")).thenReturn("set");
+ Mockito.when(secondaryRs.getString("type_full")).thenReturn(typeFull);
+ Mockito.when(secondaryRs.getString("expr")).thenReturn("col_1");
+ Mockito.when(secondaryRs.getLong("granularity")).thenReturn(1L);
+
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(connection.prepareStatement(Mockito.anyString()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(secondaryStmt);
+
+ return Assertions.assertThrows(
+ IllegalArgumentException.class, () -> ops.callGetIndexes(connection,
"db", "tbl"));
+ }
+
+ private List<Index> getIndexesForSetMetadata(String typeFull, String
expression)
+ throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement secondaryStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet secondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(secondaryRs.next()).thenReturn(true, false);
+ Mockito.when(secondaryStmt.executeQuery()).thenReturn(secondaryRs);
+
Mockito.when(secondaryRs.getString("name")).thenReturn("idx_bad_set_metadata");
+ Mockito.when(secondaryRs.getString("type")).thenReturn("set");
+ Mockito.when(secondaryRs.getString("type_full")).thenReturn(typeFull);
+ Mockito.when(secondaryRs.getString("expr")).thenReturn(expression);
+ Mockito.when(secondaryRs.getLong("granularity")).thenReturn(1L);
+
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(connection.prepareStatement(Mockito.anyString()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(secondaryStmt);
+
+ return ops.callGetIndexes(connection, "db", "tbl");
+ }
+
+ private IllegalArgumentException getIndexesFailureForSetMetadata(
+ String typeFull, String expression) throws Exception {
+ return Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
getIndexesForSetMetadata(typeFull, expression));
+ }
}
