jiangxt2 opened a new issue, #11912:
URL: https://github.com/apache/gravitino/issues/11912
### Version
main branch
### Describe what's wrong
The ClickHouse catalog only supports `minmax` and `bloom_filter` skip index
types. All other ClickHouse-supported index types (`set`, `ngrambf_v1`,
`tokenbf_v1`, `inverted`) are silently skipped on the read path and rejected on
the write path.
**Read path** — `getClickHouseIndexType()` in `ClickHouseTableOperations`
has a switch that handles only `minmax` and `bloom_filter`. The default branch
throws `IllegalArgumentException`, which is caught by the caller with a warning
log ("Skip unsupported data skipping index...") — the index is silently
dropped. Tables load successfully but index metadata is lost.
**Write path** — Two methods handle index DDL generation:
- `appendIndexesSql()`: switch handles `PRIMARY_KEY`,
`DATA_SKIPPING_MINMAX`, and `DATA_SKIPPING_BLOOM_FILTER`; all others throw
`IllegalArgumentException`
- `addIndexDefinition()`: generates only minmax and bloom_filter DDL;
`PRIMARY_KEY` is explicitly rejected with
`UnsupportedOperationException("ClickHouse does not support adding primary key
via ALTER TABLE")`; all other types throw `IllegalArgumentException`
The underlying constants are in the `IndexConstants` nested class of
`ClickHouseConstants`: only `DATA_SKIPPING_MINMAX_VALUE="minmax"` and
`DATA_SKIPPING_BLOOM_FILTER="bloom_filter"` are defined.
Note: the source code comment at `Index.IndexType` explicitly anticipates
this extension: *"The following index types are data skipping indexes in
ClickHouse, ngrambf_v1 and tokenbf_v1 Will be supported later."*
### How to reproduce
1. Create a table in ClickHouse with non-minmax/bloom indexes:
```sql
CREATE TABLE t_idx (
id Int64, val String, tags Array(String),
INDEX idx_set tags TYPE set(3) GRANULARITY 4,
INDEX idx_ngram val TYPE ngrambf_v1(3, 256, 3, 0) GRANULARITY 4,
INDEX idx_token val TYPE tokenbf_v1(256, 3, 0) GRANULARITY 4
) ENGINE=MergeTree ORDER BY id;
```
2. Load via Gravitino REST API — table loads but all three indexes are
silently dropped (warning in logs)
3. Try to create a table via Gravitino with `set` index type — throws
`IllegalArgumentException`
### Proposed fix
Add three new enum values to `Index.IndexType` in the core API:
```java
DATA_SKIPPING_SET,
DATA_SKIPPING_NGRAMFV1,
DATA_SKIPPING_TOKENBFV1,
```
This follows the established pattern — the enum already contains 11
Lance-specific values (`SCALAR`, `BTREE`, `BITMAP`, `LABEL_LIST`, `INVERTED`,
`VECTOR`, `IVF_FLAT`, `IVF_SQ`, `IVF_PQ`, `IVF_HNSW_SQ`, `IVF_HNSW_PQ`) and 2
ClickHouse-specific values (`DATA_SKIPPING_MINMAX`,
`DATA_SKIPPING_BLOOM_FILTER`).
Then extend the switch statements in `ClickHouseTableOperations` for:
- Read path: `getClickHouseIndexType()` — map ClickHouse type strings to new
enum values
- Write path: `appendIndexesSql()` and `addIndexDefinition()` — generate DDL
for new index types
Index parameters (e.g., `set(3)`, `ngrambf_v1(3, 256, 3, 0)`) are passed
through the existing `Index.properties()` mechanism.
**Scope note on `inverted`**: ClickHouse's `inverted` index is experimental
(requires `SET allow_experimental_inverted_index = 1`) and was renamed to
`full_text` in ClickHouse 24.12 / `gin` in ClickHouse 25.x. The existing
`INVERTED` enum value in `Index.IndexType` is for Lance's full-text index and
is semantically distinct from ClickHouse's data-skipping inverted index. Adding
`DATA_SKIPPING_INVERTED` is deferred to a future PR when Gravitino upgrades its
ClickHouse compatibility version and the index syntax stabilizes.
Should be coordinated with #11806 (GRANULARITY infrastructure) to avoid
conflicts in the same methods — #11806 modifies `appendIndexesSql` and
`addIndexDefinition` for GRANULARITY DDL, and this PR would reuse its planned
`Index.properties()` GRANULARITY handling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
