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 8d6591d9f0 [#11946] improvement(api): Add properties field to
TableChange.AddIndex for custom index parameters (#12013)
8d6591d9f0 is described below
commit 8d6591d9f0917841ad1a2b87a35c761da1d3da67
Author: StormSpirit <[email protected]>
AuthorDate: Wed Jul 15 19:29:40 2026 +0800
[#11946] improvement(api): Add properties field to TableChange.AddIndex for
custom index parameters (#12013)
### What changes were proposed in this pull request?
Add a `Map<String, String> properties` field to `TableChange.AddIndex`,
enabling catalog implementations to accept custom index parameters
(e.g., GRANULARITY for ClickHouse data-skipping indexes) through the
ALTER TABLE ADD INDEX path.
The change touches four layers:
- **API** (`TableChange.java`): New `addIndex(type, name, fieldNames,
properties)` factory method; existing no-properties method delegates
with `Collections.emptyMap()`. Defensive copy + null safety in
constructor. `equals`/`hashCode` updated.
- **REST DTO** (`TableUpdateRequest.java`): Pass `index.properties()`
through deserialization.
- **Core** (`ManagedTableOperations.java`): Pass
`addIndex.getProperties()` when building the new `Index` object.
- **ClickHouse catalog**: Both CREATE TABLE and ALTER TABLE paths
resolve GRANULARITY and `set(N)` max values from `index.properties()`,
falling back to sensible defaults when absent. Constants defined for
property keys (`granularity`, `set_max_values`).
### Why are the changes needed?
`TableChange.AddIndex` currently carries only `type`, `name`, and
`fieldNames` — no `properties`. This means the ALTER TABLE ADD INDEX
path cannot pass index-specific parameters to catalog implementations,
forcing them to use hardcoded defaults.
Concrete example: the ClickHouse data-skipping index support (see
#11806) allows custom GRANULARITY via `Index.properties()`. CREATE TABLE
respects user-specified GRANULARITY, but ALTER TABLE ADD INDEX was stuck
at `GRANULARITY 1` because `AddIndex` had no way to carry the value.
This PR closes that gap for both CREATE TABLE and ALTER TABLE paths.
Fix: #11946
### Does this PR introduce _any_ user-facing change?
Yes.
- New public API: `TableChange.addIndex(IndexType, String, String[][],
Map<String, String>)`. The existing 3-parameter overload is unchanged
and fully backward compatible.
- **Behavior change (ClickHouse catalog)**: bloom_filter default
GRANULARITY is unified to `1` (previously `3` in CREATE TABLE, `1` in
ALTER TABLE). This aligns with the ClickHouse server default.
### How was this patch tested?
- **Unit tests**: `TestTableChange` — 6 new tests covering properties,
null safety, equals/hashCode. `TestClickHouseTableOperations` — SQL
generation for custom properties in both CREATE TABLE and ALTER TABLE
paths, plus negative tests for invalid input.
- **Docker integration test**:
`CatalogClickHouseIT.testAlterTableAddIndexWithCustomGranularity` —
end-to-end ALTER TABLE ADD INDEX with GRANULARITY=5, verified via `SHOW
CREATE TABLE`.
Run commands:
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs`
(unit tests)
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests
"CatalogClickHouseIT" -PskipDockerTests=false` (Docker integration test)
---------
Signed-off-by: jiangxt2 <[email protected]>
---
.../java/org/apache/gravitino/rel/TableChange.java | 51 ++++-
.../java/org/apache/gravitino/TestTableChange.java | 101 ++++++++++
.../catalog/clickhouse/ClickHouseConstants.java | 6 +
.../operations/ClickHouseTableOperations.java | 95 +++++++--
.../integration/test/CatalogClickHouseIT.java | 46 +++++
.../operations/TestClickHouseTableOperations.java | 212 ++++++++++++++++++++-
.../org/apache/gravitino/client/DTOConverters.java | 3 +-
.../gravitino/dto/requests/TableUpdateRequest.java | 20 +-
.../gravitino/catalog/ManagedTableOperations.java | 1 +
docs/jdbc-clickhouse-catalog.md | 12 +-
docs/open-api/indexes.yaml | 8 +
...partitioning-distribution-sort-order-indexes.md | 13 +-
12 files changed, 533 insertions(+), 35 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/rel/TableChange.java
b/api/src/main/java/org/apache/gravitino/rel/TableChange.java
index 136f01c8a4..719a6f3f7a 100644
--- a/api/src/main/java/org/apache/gravitino/rel/TableChange.java
+++ b/api/src/main/java/org/apache/gravitino/rel/TableChange.java
@@ -22,6 +22,9 @@ package org.apache.gravitino.rel;
import com.google.common.base.Preconditions;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import org.apache.gravitino.annotation.Evolving;
@@ -450,7 +453,22 @@ public interface TableChange {
* @return A TableChange for the add index.
*/
static TableChange addIndex(IndexType type, String name, String[][]
fieldNames) {
- return new AddIndex(type, name, fieldNames);
+ return addIndex(type, name, fieldNames, Collections.emptyMap());
+ }
+
+ /**
+ * Create a TableChange for adding an index with extra properties.
+ *
+ * @param type The type of the index.
+ * @param name The name of the index.
+ * @param fieldNames The field names of the index.
+ * @param properties Extra properties for index configuration (e.g.,
granularity for ClickHouse
+ * data skipping indexes). If {@code null}, an empty map is used.
+ * @return A TableChange for the add index.
+ */
+ static TableChange addIndex(
+ IndexType type, String name, String[][] fieldNames, Map<String, String>
properties) {
+ return new AddIndex(type, name, fieldNames, properties);
}
/**
@@ -746,6 +764,7 @@ public interface TableChange {
private final String name;
private final String[][] fieldNames;
+ private final Map<String, String> properties;
/**
* @param type The type of the index.
@@ -753,9 +772,25 @@ public interface TableChange {
* @param fieldNames The field names of the index.
*/
public AddIndex(IndexType type, String name, String[][] fieldNames) {
+ this(type, name, fieldNames, Collections.emptyMap());
+ }
+
+ /**
+ * @param type The type of the index.
+ * @param name The name of the index.
+ * @param fieldNames The field names of the index.
+ * @param properties Extra properties for index configuration. A defensive
copy is made; if
+ * {@code null}, an empty map is used.
+ */
+ public AddIndex(
+ IndexType type, String name, String[][] fieldNames, Map<String,
String> properties) {
this.type = type;
this.name = name;
this.fieldNames = fieldNames;
+ this.properties =
+ properties == null
+ ? Collections.emptyMap()
+ : Collections.unmodifiableMap(new HashMap<>(properties));
}
/**
@@ -779,6 +814,13 @@ public interface TableChange {
return fieldNames;
}
+ /**
+ * @return Extra properties for index configuration. An unmodifiable map;
never {@code null}.
+ */
+ public Map<String, String> getProperties() {
+ return properties;
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) return true;
@@ -786,13 +828,14 @@ public interface TableChange {
AddIndex addIndex = (AddIndex) o;
return type == addIndex.type
&& Objects.equals(name, addIndex.name)
- && Arrays.deepEquals(fieldNames, addIndex.fieldNames);
+ && Arrays.deepEquals(fieldNames, addIndex.fieldNames)
+ && Objects.equals(properties, addIndex.properties);
}
@Override
public int hashCode() {
- int result = Objects.hash(type, name);
- result = 31 * result + Arrays.hashCode(fieldNames);
+ int result = Objects.hash(type, name, properties);
+ result = 31 * result + Arrays.deepHashCode(fieldNames);
return result;
}
}
diff --git a/api/src/test/java/org/apache/gravitino/TestTableChange.java
b/api/src/test/java/org/apache/gravitino/TestTableChange.java
index 47a0e23c35..67ab3e3978 100644
--- a/api/src/test/java/org/apache/gravitino/TestTableChange.java
+++ b/api/src/test/java/org/apache/gravitino/TestTableChange.java
@@ -25,9 +25,13 @@ import static
org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.TableChange.AddColumn;
+import org.apache.gravitino.rel.TableChange.AddIndex;
import org.apache.gravitino.rel.TableChange.ColumnPosition;
import org.apache.gravitino.rel.TableChange.DeleteColumn;
import org.apache.gravitino.rel.TableChange.RemoveProperty;
@@ -41,6 +45,7 @@ import org.apache.gravitino.rel.TableChange.UpdateColumnType;
import org.apache.gravitino.rel.TableChange.UpdateComment;
import org.apache.gravitino.rel.expressions.Expression;
import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.indexes.Index.IndexType;
import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
import org.junit.jupiter.api.Assertions;
@@ -626,4 +631,100 @@ public class TestTableChange {
assertFalse(columnB.equals(columnA));
assertNotEquals(columnA.hashCode(), columnB.hashCode());
}
+
+ @Test
+ void testAddIndexWithoutProperties() {
+ IndexType type = IndexType.PRIMARY_KEY;
+ String name = "pk_index";
+ String[][] fieldNames = new String[][] {{"col1"}};
+ AddIndex addIndex = (AddIndex) TableChange.addIndex(type, name,
fieldNames);
+
+ assertEquals(type, addIndex.getType());
+ assertEquals(name, addIndex.getName());
+ assertArrayEquals(fieldNames, addIndex.getFieldNames());
+ assertEquals(Collections.emptyMap(), addIndex.getProperties());
+ }
+
+ @Test
+ void testAddIndexWithProperties() {
+ IndexType type = IndexType.DATA_SKIPPING_BLOOM_FILTER;
+ String name = "bf_idx";
+ String[][] fieldNames = new String[][] {{"col_a"}};
+ Map<String, String> properties = new HashMap<>();
+ properties.put("granularity", "5");
+ properties.put("bloom_filter_bytes", "64");
+
+ AddIndex addIndex = (AddIndex) TableChange.addIndex(type, name,
fieldNames, properties);
+
+ assertEquals(type, addIndex.getType());
+ assertEquals(name, addIndex.getName());
+ assertArrayEquals(fieldNames, addIndex.getFieldNames());
+ assertEquals(properties, addIndex.getProperties());
+ }
+
+ @Test
+ void testAddIndexWithNullProperties() {
+ AddIndex addIndex =
+ (AddIndex)
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_MINMAX, "mm_idx", new String[][]
{{"col1"}}, null);
+
+ assertEquals(Collections.emptyMap(), addIndex.getProperties());
+ // null safety: calling getProperties() should never throw
+ assertEquals(0, addIndex.getProperties().size());
+ }
+
+ @Test
+ void testAddIndexWithEmptyProperties() {
+ AddIndex addIndex =
+ (AddIndex)
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_SET,
+ "set_idx",
+ new String[][] {{"col1"}},
+ Collections.emptyMap());
+
+ assertEquals(Collections.emptyMap(), addIndex.getProperties());
+ }
+
+ @Test
+ void testAddIndexEqualsHashCode() {
+ String[][] fields = new String[][] {{"col1"}};
+ Map<String, String> props1 = Collections.singletonMap("granularity", "1");
+ Map<String, String> props5 = Collections.singletonMap("granularity", "5");
+
+ AddIndex a1 = (AddIndex)
TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "a", fields);
+ AddIndex a2 = (AddIndex)
TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "a", fields);
+ AddIndex a3 =
+ (AddIndex) TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "a",
fields, props1);
+ AddIndex a4 =
+ (AddIndex) TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "a",
fields, props1);
+ AddIndex a5 =
+ (AddIndex) TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "a",
fields, props5);
+ AddIndex b = (AddIndex)
TableChange.addIndex(IndexType.DATA_SKIPPING_MINMAX, "b", fields);
+
+ // Reflexivity
+ assertEquals(a1, a1);
+ assertEquals(a1.hashCode(), a1.hashCode());
+
+ // Equality: same params (no props) → equal
+ assertEquals(a1, a2);
+ assertEquals(a1.hashCode(), a2.hashCode());
+
+ // Equality: same params + same props → equal
+ assertEquals(a3, a4);
+ assertEquals(a3.hashCode(), a4.hashCode());
+
+ // Inequality: same params, different props → not equal
+ assertNotEquals(a3, a5);
+
+ // Inequality: no props vs with props → not equal
+ assertNotEquals(a1, a3);
+
+ // Inequality: different name → not equal
+ assertNotEquals(a1, b);
+
+ // Null safety
+ assertFalse(a1.equals(null));
+ }
}
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 a2fca9f7b7..008de7b2aa 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
@@ -60,5 +60,11 @@ public class ClickHouseConstants {
// The name of the data skipping index type for set index in clickhouse.
public static final String DATA_SKIPPING_SET = "set";
+
+ // Key for GRANULARITY in index properties (data-skipping index
granularity).
+ public static final String GRANULARITY = "granularity";
+
+ // Key for max unique values (N) in set(N) data-skipping index properties.
+ public static final String SET_MAX_VALUES = "set_max_values";
}
}
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 688483a9ce..befc2bc761 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
@@ -21,6 +21,8 @@ package org.apache.gravitino.catalog.clickhouse.operations;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.IndexConstants.DATA_SKIPPING_BLOOM_FILTER;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.IndexConstants.DATA_SKIPPING_MINMAX_VALUE;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.IndexConstants.DATA_SKIPPING_SET;
+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.CLICKHOUSE_ENGINE_KEY;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE_PROPERTY_ENTRY;
import static
org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.GRAVITINO_ENGINE_KEY;
@@ -484,27 +486,34 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
sqlBuilder.append(" PRIMARY KEY (").append(fieldStr).append(")");
break;
case DATA_SKIPPING_MINMAX:
- // The GRANULARITY value is always 1 here currently as we can't set
it by Index: there is
- // no field for it.
- // TODO(yuqi) add a properties field to Index to support user
defined GRANULARITY value.
sqlBuilder
.append(" ")
- .append(buildDataSkippingIndexDdl(index.name(), fieldStr,
"minmax", 1));
+ .append(
+ buildDataSkippingIndexDdl(
+ index.name(),
+ fieldStr,
+ DATA_SKIPPING_MINMAX_VALUE,
+ resolveGranularity(index.properties(), 1)));
break;
case DATA_SKIPPING_BLOOM_FILTER:
- // The GRANULARITY value is always 3 here currently.
- // TODO(yuqi) add a properties field to Index to support user
defined GRANULARITY value.
sqlBuilder
.append(" ")
- .append(buildDataSkippingIndexDdl(index.name(), fieldStr,
"bloom_filter", 3));
+ .append(
+ buildDataSkippingIndexDdl(
+ index.name(),
+ fieldStr,
+ DATA_SKIPPING_BLOOM_FILTER,
+ resolveGranularity(index.properties(), 1)));
break;
case DATA_SKIPPING_SET:
- // The max unique values (N) is always 0 (unlimited) here currently
as we can't set it
- // by Index: there is no field for it. ClickHouse requires set(N)
syntax.
- // TODO(yuqi) add a properties field to Index to support user
defined max unique values.
sqlBuilder
.append(" ")
- .append(buildDataSkippingIndexDdl(index.name(), fieldStr,
"set(0)", 1));
+ .append(
+ buildDataSkippingIndexDdl(
+ index.name(),
+ fieldStr,
+ "set(" + resolveSetMaxValues(index.properties()) + ")",
+ resolveGranularity(index.properties(), 1)));
break;
default:
throw new IllegalArgumentException(
@@ -874,18 +883,31 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
Preconditions.checkArgument(!indexExists, "Index '%s' already exists",
addIndex.getName());
String fieldStr = getIndexFieldStr(addIndex.getFieldNames());
+ Map<String, String> properties = addIndex.getProperties();
switch (addIndex.getType()) {
case DATA_SKIPPING_MINMAX:
- return "ADD " + buildDataSkippingIndexDdl(addIndex.getName(),
fieldStr, "minmax", 1);
+ return "ADD "
+ + buildDataSkippingIndexDdl(
+ addIndex.getName(),
+ fieldStr,
+ DATA_SKIPPING_MINMAX_VALUE,
+ resolveGranularity(properties, 1));
case DATA_SKIPPING_BLOOM_FILTER:
- return "ADD " + buildDataSkippingIndexDdl(addIndex.getName(),
fieldStr, "bloom_filter", 3);
+ return "ADD "
+ + buildDataSkippingIndexDdl(
+ addIndex.getName(),
+ fieldStr,
+ DATA_SKIPPING_BLOOM_FILTER,
+ resolveGranularity(properties, 1));
case DATA_SKIPPING_SET:
- // The max unique values (N) is always 0 (unlimited) here currently as
we can't set it
- // by Index: there is no field for it. ClickHouse requires set(N)
syntax.
- // TODO(yuqi) add a properties field to Index to support user defined
max unique values.
- return "ADD " + buildDataSkippingIndexDdl(addIndex.getName(),
fieldStr, "set(0)", 1);
+ return "ADD "
+ + buildDataSkippingIndexDdl(
+ addIndex.getName(),
+ fieldStr,
+ "set(" + resolveSetMaxValues(properties) + ")",
+ resolveGranularity(properties, 1));
case PRIMARY_KEY:
throw new UnsupportedOperationException(
@@ -897,6 +919,45 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
}
}
+ /**
+ * Resolves an integer property from the index properties map.
+ *
+ * @param properties the index properties map
+ * @param key the property key (e.g. {@link
ClickHouseConstants.IndexConstants#GRANULARITY})
+ * @param defaultValue the value returned when the key is absent
+ * @param minValue the minimum allowed value (inclusive)
+ * @return the resolved integer value
+ * @throws IllegalArgumentException if the value is present but not a valid
integer within bounds
+ */
+ private int resolveIntProperty(
+ Map<String, String> properties, String key, int defaultValue, int
minValue) {
+ if (properties == null) {
+ return defaultValue;
+ }
+ String raw = properties.get(key);
+ if (raw == null) {
+ return defaultValue;
+ }
+ raw = raw.trim();
+ int value;
+ try {
+ value = Integer.parseInt(raw);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(key + " must be a valid integer, but
got: " + raw, e);
+ }
+ Preconditions.checkArgument(
+ value >= minValue, "%s must be >= %s, but got: %s", key, minValue,
value);
+ return value;
+ }
+
+ private int resolveGranularity(Map<String, String> properties, int
defaultValue) {
+ return resolveIntProperty(properties, GRANULARITY, defaultValue, 1);
+ }
+
+ private int resolveSetMaxValues(Map<String, String> properties) {
+ return resolveIntProperty(properties, SET_MAX_VALUES, 0, 0);
+ }
+
@VisibleForTesting
private String deleteIndexDefinition(
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
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 a1f64dacda..6fa6f470ca 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
@@ -1509,6 +1509,52 @@ public class CatalogClickHouseIT extends BaseIT {
autoIncrementFalseException.getMessage().contains("auto increment is
not supported"));
}
+ @Test
+ void testAlterTableAddIndexWithCustomGranularity() {
+ String tableName = GravitinoITUtils.genRandomName("alter_idx_gran");
+ NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, tableName);
+ Column[] columns =
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), "id", false, false,
DEFAULT_VALUE_NOT_SET),
+ Column.of("val", Types.IntegerType.get(), "val", false, false,
DEFAULT_VALUE_NOT_SET)
+ };
+ TableCatalog tableCatalog = catalog.asTableCatalog();
+ tableCatalog.createTable(
+ tableIdentifier,
+ columns,
+ table_comment,
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ getSortOrders("id"));
+
+ // ALTER TABLE ADD INDEX with custom GRANULARITY via AddIndex properties
+ tableCatalog.alterTable(
+ tableIdentifier,
+ TableChange.addIndex(
+ Index.IndexType.DATA_SKIPPING_MINMAX,
+ "idx_val_minmax",
+ new String[][] {{"val"}},
+ Collections.singletonMap("granularity", "5")));
+
+ Table loaded = tableCatalog.loadTable(tableIdentifier);
+ Assertions.assertTrue(
+ Arrays.stream(loaded.index())
+ .anyMatch(
+ index ->
+ Objects.equals(index.name(), "idx_val_minmax")
+ && index.type() ==
Index.IndexType.DATA_SKIPPING_MINMAX));
+
+ // Verify DDL-level: SHOW CREATE TABLE should contain GRANULARITY 5
+ String createSql =
+ clickhouseService.executeQueryForResult(
+ String.format("SHOW CREATE TABLE `%s`.`%s`", schemaName,
tableName));
+ Assertions.assertNotNull(createSql);
+ Assertions.assertTrue(
+ createSql.contains("GRANULARITY 5"),
+ "SHOW CREATE TABLE output should contain GRANULARITY 5: " + createSql);
+ }
+
@Test
void testCreateTableWithAutoIncrementUnsupported() {
String tableName = GravitinoITUtils.genRandomName("create_auto_inc");
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 3a54b45812..635c41f021 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
@@ -1115,10 +1115,74 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Assertions.assertTrue(sql.contains("PARTITION BY `c1`"));
Assertions.assertTrue(sql.contains("INDEX `idx_c2` `c2` TYPE minmax
GRANULARITY 1"));
- Assertions.assertTrue(sql.contains("INDEX `idx_c3` `c3` TYPE bloom_filter
GRANULARITY 3"));
+ Assertions.assertTrue(sql.contains("INDEX `idx_c3` `c3` TYPE bloom_filter
GRANULARITY 1"));
Assertions.assertTrue(sql.contains("INDEX `idx_c4` `c2` TYPE set(0)
GRANULARITY 1"));
}
+ @Test
+ void testGenerateCreateTableSqlWithCustomIndexProperties() {
+ TestableClickHouseTableOperations ops = new
TestableClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+
+ JdbcColumn[] cols =
+ new JdbcColumn[] {
+ JdbcColumn.builder()
+ .withName("c1")
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .build(),
+ JdbcColumn.builder()
+ .withName("c2")
+ .withType(Types.StringType.get())
+ .withNullable(true)
+ .build(),
+ };
+
+ Index[] indexes =
+ new Index[] {
+ Indexes.of(
+ IndexType.DATA_SKIPPING_MINMAX,
+ "idx_mm",
+ new String[][] {{"c1"}},
+ Collections.singletonMap("granularity", "5")),
+ Indexes.of(
+ IndexType.DATA_SKIPPING_BLOOM_FILTER,
+ "idx_bf",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("granularity", "10")),
+ Indexes.of(
+ IndexType.DATA_SKIPPING_SET,
+ "idx_set",
+ new String[][] {{"c2"}},
+ new HashMap<String, String>() {
+ {
+ put("set_max_values", "100");
+ put("granularity", "3");
+ }
+ }),
+ };
+
+ String sql =
+ ops.buildCreateSql(
+ "t_idx_custom",
+ cols,
+ "comment",
+ new HashMap<>(),
+ new Transform[] {},
+ Distributions.NONE,
+ indexes,
+ ClickHouseUtils.getSortOrders("c1"));
+
+ Assertions.assertTrue(sql.contains("INDEX `idx_mm` `c1` TYPE minmax
GRANULARITY 5"));
+ Assertions.assertTrue(sql.contains("INDEX `idx_bf` `c2` TYPE bloom_filter
GRANULARITY 10"));
+ Assertions.assertTrue(sql.contains("INDEX `idx_set` `c2` TYPE set(100)
GRANULARITY 3"));
+ }
+
@Test
void testGenerateCreateTableSqlWithAutoIncrementColumnUnsupported() {
TestableClickHouseTableOperations ops = new
TestableClickHouseTableOperations();
@@ -1380,7 +1444,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
IndexType.DATA_SKIPPING_BLOOM_FILTER, "idx_bf", new
String[][] {{"c2"}})
});
Assertions.assertTrue(
- bloomSql.contains("ADD INDEX `idx_bf` `c2` TYPE bloom_filter
GRANULARITY 3"));
+ bloomSql.contains("ADD INDEX `idx_bf` `c2` TYPE bloom_filter
GRANULARITY 1"));
String setSql =
ops.buildAlterSql(
@@ -1413,6 +1477,150 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
}));
}
+ @Test
+ public void testAlterTableAddIndexWithCustomGranularity() {
+ StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+ ops.setTable(buildStubTable());
+
+ // Custom GRANULARITY for minmax
+ String minmaxSql =
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_MINMAX,
+ "idx_mm",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("granularity", "5"))
+ });
+ Assertions.assertTrue(minmaxSql.contains("ADD INDEX `idx_mm` `c2` TYPE
minmax GRANULARITY 5"));
+
+ // Custom GRANULARITY for bloom_filter
+ String bloomSql =
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_BLOOM_FILTER,
+ "idx_bf",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("granularity", "10"))
+ });
+ Assertions.assertTrue(
+ bloomSql.contains("ADD INDEX `idx_bf` `c2` TYPE bloom_filter
GRANULARITY 10"));
+
+ // Custom set_max_values for set index
+ String setSql =
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_SET,
+ "idx_set",
+ new String[][] {{"c2"}},
+ new HashMap<>() {
+ {
+ put("set_max_values", "100");
+ put("granularity", "3");
+ }
+ })
+ });
+ Assertions.assertTrue(setSql.contains("ADD INDEX `idx_set` `c2` TYPE
set(100) GRANULARITY 3"));
+ }
+
+ @Test
+ public void testAlterTableAddIndexInvalidGranularity() {
+ StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+ ops.setTable(buildStubTable());
+
+ // Non-numeric granularity
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_MINMAX,
+ "idx1",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("granularity", "abc"))
+ }));
+
+ // Zero granularity
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_BLOOM_FILTER,
+ "idx2",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("granularity", "0"))
+ }));
+ }
+
+ @Test
+ public void testAlterTableAddIndexInvalidSetMaxValues() {
+ StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+ ops.setTable(buildStubTable());
+
+ // Non-numeric set_max_values
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_SET,
+ "idx1",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("set_max_values", "abc"))
+ }));
+
+ // Negative set_max_values
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {
+ TableChange.addIndex(
+ IndexType.DATA_SKIPPING_SET,
+ "idx2",
+ new String[][] {{"c2"}},
+ Collections.singletonMap("set_max_values", "-1"))
+ }));
+ }
+
@Test
public void testGetClickHouseIndexType() {
StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
index 781331a553..b681bb6dea 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
@@ -235,7 +235,8 @@ class DTOConverters {
return new TableUpdateRequest.AddTableIndexRequest(
((TableChange.AddIndex) change).getType(),
((TableChange.AddIndex) change).getName(),
- ((TableChange.AddIndex) change).getFieldNames());
+ ((TableChange.AddIndex) change).getFieldNames(),
+ ((TableChange.AddIndex) change).getProperties());
} else if (change instanceof TableChange.DeleteIndex) {
return new TableUpdateRequest.DeleteTableIndexRequest(
((TableChange.DeleteIndex) change).getName(),
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
index eb02c4cfe8..300c15c3cb 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
@@ -29,6 +29,8 @@ import
com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.google.common.base.Preconditions;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
import javax.annotation.Nullable;
import lombok.EqualsAndHashCode;
import lombok.Getter;
@@ -818,7 +820,20 @@ public interface TableUpdateRequest extends RESTRequest {
* @param fieldNames The field names under the table contained in the
index.
*/
public AddTableIndexRequest(Index.IndexType type, String name, String[][]
fieldNames) {
- this.index = Indexes.of(type, name, fieldNames);
+ this(type, name, fieldNames, Collections.emptyMap());
+ }
+
+ /**
+ * The constructor of the add table index request with extra properties.
+ *
+ * @param type The type of the index
+ * @param name The name of the index
+ * @param fieldNames The field names under the table contained in the
index.
+ * @param properties Extra properties for index configuration
+ */
+ public AddTableIndexRequest(
+ Index.IndexType type, String name, String[][] fieldNames, Map<String,
String> properties) {
+ this.index = Indexes.of(type, name, fieldNames, properties);
}
/**
@@ -840,7 +855,8 @@ public interface TableUpdateRequest extends RESTRequest {
*/
@Override
public TableChange tableChange() {
- return TableChange.addIndex(index.type(), index.name(),
index.fieldNames());
+ return TableChange.addIndex(
+ index.type(), index.name(), index.fieldNames(), index.properties());
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
index fe761e7962..0e226abcbb 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
@@ -250,6 +250,7 @@ public abstract class ManagedTableOperations implements
TableCatalog {
.withName(addIndex.getName())
.withFieldNames(addIndex.getFieldNames())
.withIndexType(addIndex.getType())
+ .withProperties(addIndex.getProperties())
.build();
newIndexes.add(newIndex);
diff --git a/docs/jdbc-clickhouse-catalog.md b/docs/jdbc-clickhouse-catalog.md
index 4075ff2b38..84964e2e13 100644
--- a/docs/jdbc-clickhouse-catalog.md
+++ b/docs/jdbc-clickhouse-catalog.md
@@ -172,7 +172,7 @@ See [Manage Relational Metadata Using
Gravitino](./manage-relational-metadata-us
| Mapping | Gravitino table maps to a ClickHouse table
|
| Engines | **MergeTree family** (`MergeTree` default,
`ReplacingMergeTree`, `SummingMergeTree`, `AggregatingMergeTree`,
`CollapsingMergeTree`, `VersionedCollapsingMergeTree`, `GraphiteMergeTree`):
fully supported, data persists across restarts. **Log family** (`TinyLog`,
`StripeLog`, `Log`): supported, data and table definition persist across
restarts. **`Null`**: supported, table persists, data is always discarded by
design. **`Set`**: supported, table definition persists. [...]
| Ordering/Partition | MergeTree-family requires exactly one `ORDER BY`
column; only single-column identity `PARTITION BY` is supported on MergeTree
engines. Other engines reject `ORDER BY`/`PARTITION BY`.
|
-| Indexes | Primary key; data-skipping indexes
`DATA_SKIPPING_MINMAX`, `DATA_SKIPPING_BLOOM_FILTER`, and `DATA_SKIPPING_SET`
(fixed granularities).
|
+| Indexes | Primary key; data-skipping indexes
`DATA_SKIPPING_MINMAX`, `DATA_SKIPPING_BLOOM_FILTER`, and `DATA_SKIPPING_SET`
(configurable granularity via `Index.properties()`).
[...]
| Distribution | Gravitino enforces `Distributions.NONE`; no custom
distribution strategies.
|
| Column defaults | Supported.
|
| Unsupported | Engine change after creation; removing table
properties; auto-increment columns.
|
@@ -240,9 +240,11 @@ If you need Gravitino to manage an existing cluster
database or table, recreate
- `PRIMARY_KEY`
- Data-skipping indexes:
- - `DATA_SKIPPING_MINMAX` (`GRANULARITY` fixed to 1)
- - `DATA_SKIPPING_BLOOM_FILTER` (`GRANULARITY` fixed to 3)
- - `DATA_SKIPPING_SET` (`GRANULARITY` fixed to 1)
+ - `DATA_SKIPPING_MINMAX` (default `GRANULARITY 1`)
+ - `DATA_SKIPPING_BLOOM_FILTER` (default `GRANULARITY 1`)
+ - `DATA_SKIPPING_SET` (default `GRANULARITY 1`, plus configurable `set(N)`
max values)
+
+ Custom `GRANULARITY` can be specified via the `Index.properties()` API (key
`granularity`, value must be a positive integer). For `DATA_SKIPPING_SET`, the
max unique values can be configured via `set_max_values` (non-negative
integer). If not specified, the defaults above apply.
### Partitioning, Sorting, and Distribution
@@ -348,7 +350,7 @@ Supported:
- Rename column.
- Update column type/comment/default/position/nullability.
- Delete columns (with `IF EXISTS` support).
-- Add data-skipping indexes; drop data-skipping indexes. Adding/dropping
primary key is not supported.
+- Add data-skipping indexes with custom `GRANULARITY` and `set(N)` via
`Index.properties()`; drop data-skipping indexes. Adding/dropping primary key
is not supported.
- Update table comment.
Unsupported:
diff --git a/docs/open-api/indexes.yaml b/docs/open-api/indexes.yaml
index ea2fe90edf..c4e1a00cf2 100644
--- a/docs/open-api/indexes.yaml
+++ b/docs/open-api/indexes.yaml
@@ -35,9 +35,17 @@ components:
enum:
- "primary_key"
- "unique_key"
+ - "data_skipping_minmax"
+ - "data_skipping_bloom_filter"
+ - "data_skipping_set"
name:
type: string
description: The name of the index
nullable: true
fieldNames:
$ref: "./tables.yaml#/components/schemas/FieldNames"
+ properties:
+ type: object
+ additionalProperties:
+ type: string
+ description: Extra index properties (e.g., granularity for
ClickHouse data-skipping indexes)
diff --git a/docs/table-partitioning-distribution-sort-order-indexes.md
b/docs/table-partitioning-distribution-sort-order-indexes.md
index 022ef204f7..65025ec836 100644
--- a/docs/table-partitioning-distribution-sort-order-indexes.md
+++ b/docs/table-partitioning-distribution-sort-order-indexes.md
@@ -245,7 +245,7 @@ tableCatalog.createTable(
## Indexes
-To define an indexed table, you should utilize the following three components
to construct a valid indexed table:
+To define an indexed table, you should utilize the following four components
to construct a valid indexed table:
- IndexType. Represents the type of index, such as primary key or unique key.
@@ -258,6 +258,8 @@ To define an indexed table, you should utilize the
following three components to
- FieldNames. It defines which table fields Gravitino uses to index the table.
+- Properties (optional). A map of extra index configuration properties (e.g.,
`granularity` for ClickHouse data-skipping indexes). If omitted, an empty map
is used.
+
<Tabs groupId='language' queryString>
<TabItem value="Json" label="JSON">
@@ -265,7 +267,8 @@ To define an indexed table, you should utilize the
following three components to
{
"indexType": "PRIMARY_KEY",
"name": "PRIMARY",
- "fieldNames": [["col_1"],["col_2"]]
+ "fieldNames": [["col_1"],["col_2"]],
+ "properties": {}
}
```
@@ -319,12 +322,14 @@ curl -X POST -H "Accept:
application/vnd.gravitino.v1+json" \
{
"indexType": "PRIMARY_KEY",
"name": "PRIMARY",
- "fieldNames": [["id"]]
+ "fieldNames": [["id"]],
+ "properties": {}
},
{
"indexType": "UNIQUE_KEY",
"name": "name_age_score_uk",
- "fieldNames": [["name"],["age"],["score]]
+ "fieldNames": [["name"],["age"],["score"]],
+ "properties": {}
}
]
}'
http://localhost:8090/api/metalakes/metalake/catalogs/catalog/schemas/schema/tables
