jiangxt2 opened a new pull request, #12917:
URL: https://github.com/apache/gravitino/pull/12917

   ### What changes were proposed in this pull request?
   
   This pull request stops the ClickHouse index metadata parser from 
recursively removing function wrappers. Direct column identifiers and tuples 
containing only identifiers retain their existing behavior, while function 
expressions such as `lower(name)` are rejected by the existing field validation 
and handled by the existing per-index warning-and-skip path.
   
   The previous wrapper-unwrapping behavior is retained in a 
sharding-key-specific helper so function-wrapped sharding keys such as 
`cityHash64(user_id)` remain unchanged. Unsupported index warnings retain the 
index, table, and type context without logging the raw expression, `type_full`, 
or parser exception.
   
   Bloom-filter parameter parsing remains outside this skip boundary, so 
malformed parameter metadata continues to fail with index context.
   
   Unit coverage verifies rejection of direct and nested function expressions, 
including hash functions, and mixed tuples, while retaining existing 
arithmetic-expression rejection and simple/tuple compatibility coverage. A 
focused ClickHouse integration test verifies that a native expression index 
remains present in the source DDL, is not exposed as a different column index, 
and is not recreated with changed semantics.
   
   ### Why are the changes needed?
   
   Gravitino's public `Index.fieldNames()` contract can represent column 
references but not arbitrary ClickHouse expressions. Reporting `lower(name)` as 
the field `name` fabricates metadata and causes load-to-create round trips to 
produce a semantically different index. Skipping the unsupported index 
preserves metadata correctness without adding a new expression API or SQL 
parser.
   
   Fix: #12915
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. Loading a ClickHouse table no longer exposes a function-based 
data-skipping index as a plain-column Gravitino index. Unsupported expression 
indexes are omitted with a warning while the table remains loadable. Simple 
identifier and identifier-only tuple indexes are unchanged, and no public API 
or property key is added or removed.
   
   ### How was this patch tested?
   
   - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:spotlessCheck 
--console=plain` - passed.
   - `./gradlew rat --console=plain` - passed.
   - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs 
--console=plain` - passed with 100 tests, 0 skipped, 0 failures, and 0 errors.
   - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 
'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseIT.testLoadExpressionIndexDoesNotFabricateColumnIndex'
 -PskipDockerTests=false --console=plain --no-daemon` - passed against 
ClickHouse 24.8.14 with 1 test, 0 skipped, 0 failures, and 0 errors.
   - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test --tests 
'org.apache.gravitino.catalog.clickhouse.integration.test.CatalogClickHouseClusterIT'
 -PskipDockerTests=false --console=plain --no-daemon` - passed against 
ClickHouse 24.8.14 with 19 tests, 0 skipped, 0 failures, and 0 errors.
   - `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build -x test 
--console=plain` - passed.
   - `git diff --check` - passed.
   


-- 
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]

Reply via email to