jiangxt2 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4245479592
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableSqlUtils.java:
##########
@@ -577,6 +587,21 @@ private static String extractPartitionIdentifier(String
expression) {
return isStrictIdentifier(trimmed) ? trimmed : null;
}
+ /**
+ * Resolves the partition column for a known granularity function. When the
inner expression is
+ * too complex to resolve as a simple column name and exactly one column
participates in the
+ * partition, that column is used as a fallback.
+ */
+ @Nullable
+ private static String resolvePartitionIdentifier(
+ String expression, List<String> partitionColumns) {
+ String identifier = extractSimpleIdentifier(expression);
+ if (identifier != null) {
+ return identifier;
+ }
+ return partitionColumns.size() == 1 ? partitionColumns.get(0) : null;
Review Comment:
**[P1] Single-column fallback changes partition semantics**
A single partition-key dependency does not imply semantic equivalence. This
fallback maps `toYYYYMM(d + 1)` to `month("d")`, dropping the offset. It also
removes required casts for String/UInt32 columns, producing expressions
ClickHouse rejects. Verified through this PR’s `load()` on Docker ClickHouse
24.8.14.39. Please only simplify equivalent expressions with compatible source
types; otherwise retain empty `partitioning()` and the original
`partition-key`, and add regression tests.
```sql
WITH toDate('2026-01-31') AS d
SELECT toYYYYMM(d + 1) AS native_key, toYYYYMM(d) AS reconstructed_key;
-- 202602 | 202601
```
--
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]
