jiangxt2 opened a new pull request, #11806:
URL: https://github.com/apache/gravitino/pull/11806
## What changes were proposed in this pull request?
### GRANULARITY Support (`ClickHouseTableOperations.java`)
- **Write path**: `appendIndexesSql()` now reads GRANULARITY from
`Index.properties()` via new `resolveGranularity()` method, falling back to
ClickHouse's default of `1` when not specified. Previously, GRANULARITY was
hardcoded (minmax=1, bloom_filter=3).
- **Read path**: `getSecondaryIndexes()` now queries the `granularity`
column from `system.data_skipping_indices` and stores it in
`Index.properties()` via `Indexes.of(type, name, fields, Map.of("granularity",
value))`. Previously, GRANULARITY was silently lost on load.
- **Validation**: `resolveGranularity()` validates the value is a
non-negative integer before DDL interpolation, preventing malformed SQL.
### Shard Key Validation (`ClickHouseTableOperations.java`)
- Added `nullable` check: shard key column must not be nullable
- Added type check: shard key column's data type must implement
`Type.IntegralType` (covers `ByteType`, `ShortType`, `IntegerType`, `LongType`
and unsigned variants)
- Client-side validation provides clear error messages instead of opaque
`TYPE_MISMATCH` from ClickHouse server
- Type/nullability checks only apply to bare-column shard keys;
function-wrapped keys (e.g. `cityHash64(string_col)`) are accepted regardless
of inner column type, since hash functions return valid integer results
## Does this PR introduce any user-facing change?
1. Users can now specify custom GRANULARITY for data skipping indexes via
`Index.properties()`. The round-trip (create → load → recreate) preserves the
GRANULARITY value. Note: `TableChange.AddIndex` API has no `properties` field,
so custom GRANULARITY cannot be passed through ALTER TABLE ADD INDEX — only the
default is used.
2. Default GRANULARITY for bloom_filter indexes changed from 3 to 1,
aligning with ClickHouse's actual default.
3. Attempting to create a distributed table with a nullable or non-integer
shard key now produces a clear error message from Gravitino.
## How was this patch tested?
Unit tests:
- `testGenerateCreateTableSqlWithCustomGranularity`: Custom GRANULARITY
values (5, 10) in CREATE TABLE SQL
- `testIndexNonNumericGranularityRejected`: Non-numeric GRANULARITY value
rejected
- `testShardingKeyNullableColumnRejected`: Nullable integer column rejected
as shard key
- `testShardingKeyNonIntegerColumnRejected`: String column rejected as shard
key
- `testShardingKeyFunctionWithNonIntegerColumnAccepted`:
cityHash64(string_col) accepted
- `testShardingKeyInt128ColumnCurrentlyRejected`: Int128 bare column
rejected (documented limitation)
Integration tests (ClickHouse):
- `testCreateAndLoadWithCustomGranularity`: Create table with minmax
GRANULARITY=5 and bloom_filter GRANULARITY=10, load table, verify
`Index.properties()` preserves the values
Closes #11802
--
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]
