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]
