jiangxt2 opened a new issue, #11946: URL: https://github.com/apache/gravitino/issues/11946
### What would you like to be improved? `TableChange.AddIndex` (`api/src/main/java/org/apache/gravitino/rel/TableChange.java`) only carries three fields: - `Index.IndexType type` - `String name` - `String[][] fieldNames` There is no `properties` field. This means the `ALTER TABLE ADD INDEX` path cannot pass any index-specific parameters to catalog implementations. The `Index` interface already supports `properties()` (`Index.java`), and `Indexes.of(type, name, fieldNames, properties)` works correctly in the CREATE TABLE path. But when a catalog processes `TableChange.AddIndex` in `alterTable()`, properties are unavailable — it must fall back to hardcoded defaults. **Concrete example:** PR #11806 added customizable GRANULARITY for ClickHouse data-skipping indexes via `Index.properties()`. CREATE TABLE respects user-specified GRANULARITY, but ALTER TABLE ADD INDEX always generates `GRANULARITY 1` because `AddIndex` has no way to carry the value. This gap was identified during review of #11806 (https://github.com/apache/gravitino/pull/11806#discussion_r3544289778). ### How should we improve? Add a `Map<String, String> properties` field to `TableChange.AddIndex`: 1. **AddIndex class** (`TableChange.java`): - Add `private final Map<String, String> properties` field - Add constructor overload accepting properties - Update `equals()` / `hashCode()` to include properties 2. **Factory method** (`TableChange.java`): - Add overload: `addIndex(IndexType type, String name, String[][] fieldNames, Map<String, String> properties)` - Existing `addIndex(IndexType, String, String[][])` delegates with `Collections.emptyMap()` 3. **REST API serialization** (TableChangeDTO or equivalent): - Add optional `properties` field; absent/null treated as empty map for backward compatibility 4. **Catalog implementations**: - Update ClickHouse `addIndexDefinition()` to read `addIndex.properties()` when generating ALTER TABLE ADD INDEX (falling back to default when absent) - Other catalogs remain unaffected (empty map = use catalog default) ### Related - PR #11806 known limitation: https://github.com/apache/gravitino/pull/11806 - `Index.properties()`: `api/src/main/java/org/apache/gravitino/rel/indexes/Index.java` - `Indexes.of()` with properties: `api/src/main/java/org/apache/gravitino/rel/indexes/Indexes.java` -- 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]
