jiangxt2 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4441111564
##########
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:
**[P2] Preserve narrowing conversions for `DateTime64`**
`TimestampType` also represents `DateTime64`, so this accepts
`toYear(toDateTime(ts))` as `year(ts)`, although the conversion is not
necessarily lossless. Confirmed on ClickHouse 24.8.14.39 with `UTC` and
`date_time_overflow_behavior='ignore'`: for a `DateTime64(0)` column containing
`1969-12-31 00:00:00`, the original expression returns 2106, while `toYear(ts)`
returns 1969. This exposes a structured partition transform that is not
equivalent to the original partition key. Please retain native source-type
information and check the conversion direction; when equivalence cannot be
established, fall back to unstructured partitioning while preserving the raw
key. Please also add this boundary case through the actual
type-conversion/parser path.
--
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]
