cwq222 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4442656416
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableSqlUtils.java:
##########
@@ -577,6 +587,41 @@ private static String extractPartitionIdentifier(String
expression) {
return isStrictIdentifier(trimmed) ? trimmed : null;
}
+ /**
+ * Resolves the partition column for a granularity function. A simple column
name is returned
+ * directly. Otherwise a pure type conversion (toDate/toDateTime) over a
Date/DateTime column is
+ * unwrapped, which is semantically lossless for the partition granularity.
+ */
+ @Nullable
+ private static String resolvePartitionIdentifier(
+ String expression, Map<String, Type> columnTypes) {
+ String identifier = extractSimpleIdentifier(expression);
+ if (identifier != null) {
+ return identifier;
+ }
+ return unwrapPureConversion(expression, columnTypes);
+ }
+
+ /**
+ * Unwraps a single pure type conversion (toDate/toDateTime) over a
Date/DateTime column. Other
+ * expressions (arithmetic, casts, nested functions) are not unwrapped
because doing so would
+ * change the partition semantics.
+ */
+ @Nullable
+ private static String unwrapPureConversion(String expression, Map<String,
Type> columnTypes) {
+ String trimmed = StringUtils.trim(expression);
+ Matcher matcher = TYPE_CONVERSION_PATTERN.matcher(trimmed);
+ if (!matcher.matches()) {
+ return null;
+ }
+ String inner = extractSimpleIdentifier(matcher.group(2));
+ if (inner == null) {
+ return null;
+ }
+ Type type = columnTypes.get(inner);
+ return type instanceof Types.DateType || type instanceof
Types.TimestampType ? inner : null;
Review Comment:
Thanks for catching this. I've replaced the `TimestampType` check with
ClickHouse's native `TYPE_NAME` and now check the conversion direction:
`toDate(col)` unwraps only for `Date`/`DateTime`, and `toDateTime(col)` only
for `DateTime`. Narrowing conversions (`DateTime64` -> `DateTime`, `Date` ->
`DateTime`) stay unstructured with the raw expression preserved in
`partition-key`.
Added boundary cases (two-arg overload with real `TYPE_NAME`) and an IT
covering both narrowing cases.
--
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]
