jiangxt2 opened a new issue, #11802:
URL: https://github.com/apache/gravitino/issues/11802
### What would you like to be improved?
There are two issues in the ClickHouse catalog's index and distributed table
handling. Both are in the same file and involve pending TODO items from the
same author, so I'm filing them together. Happy to split if preferred.
**1. Index GRANULARITY hardcoded with wrong default for bloom_filter**
`ClickHouseTableOperations.java` hardcodes GRANULARITY values in three
locations:
- `appendIndexesSql()` (line 486): minmax → `GRANULARITY 1`
- `appendIndexesSql()` (line 495): bloom_filter → `GRANULARITY 3`
- `addIndexDefinition()` (line 841, 845): same hardcoded values
The bloom_filter default of `3` is incorrect — ClickHouse's actual default
is `1`. This means tables created through Gravitino have different index
behavior than tables created directly via SQL.
Additionally, `getSecondaryIndexes()` (line 1243) queries
`system.data_skipping_indices` but does not read the `granularity` column,
losing round-trip fidelity.
The TODO comments at lines 484 and 491 acknowledge this gap: `// TODO(yuqi)
add a properties field to Index to support user defined GRANULARITY value.`
The `Index.properties()` API (Index.java line 52) and `Indexes.of(type,
name, fields, properties)` already exist — the infrastructure is ready.
**2. Shard key validation only checks column existence, not type or
nullability**
`ClickHouseTableOperations.java` `handleDistributeTable()` (line 348-352)
has a TODO: `// TODO(yuqi) WE need to check the columns in shard keys should be
integer and not nullable`.
Current validation (lines 359-369) only checks that the column name exists.
ClickHouse itself rejects non-integer and nullable columns with
`TYPE_MISMATCH`, but the error message is opaque. Client-side validation would
provide clearer feedback.
Verified on a local ClickHouse cluster:
- `Nullable(Int64)` as shard key → server error: `Sharding expression has
type Nullable(Int64), but should be one of integer type`
- `String` as shard key → server error: `Sharding expression has type
String, but should be one of integer type`
### How should we improve?
**For GRANULARITY (3 locations in ClickHouseTableOperations.java):**
- Read `index.properties().getOrDefault("granularity", "1")` instead of
hardcoded values
- In `getSecondaryIndexes()`, add `granularity` to the SELECT query and pass
it via `Indexes.of(type, name, fields, Map.of("granularity", value))`
- Fix bloom_filter default from `3` to `1` (ClickHouse actual default)
**For shard key validation (handleDistributeTable):**
- After finding the column, add:
```java
Preconditions.checkArgument(!col.nullable(),
"Sharding key column '%s' must not be nullable", columnName);
Preconditions.checkArgument(isIntegerType(col.dataType()),
"Sharding key column '%s' must be an integer type", columnName);
```
--
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]
