cwq222 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4451521938
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableSqlUtils.java:
##########
@@ -577,6 +589,55 @@ 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 or DateTime column
+ * is unwrapped, which is semantically lossless for the partition
granularity.
+ */
+ @Nullable
+ private static String resolvePartitionIdentifier(
+ String expression, Map<String, String> columnTypeNames) {
+ String identifier = extractSimpleIdentifier(expression);
+ if (identifier != null) {
+ return identifier;
+ }
+ return unwrapPureConversion(expression, columnTypeNames);
+ }
+
+ /**
+ * Unwraps a single pure type conversion (toDate/toDateTime) only when the
conversion is lossless
+ * for the partition granularity. The conversion direction is checked
against the source column
+ * type: toDate is lossless for Date (identity) and DateTime (drops the
time-of-day, the date part
+ * is unchanged); toDateTime is lossless only for DateTime. Other
combinations, such as toDateTime
+ * over Date or DateTime64, are narrowing conversions that can overflow, so
they are not
+ * unwrapped. Arithmetic, casts, and nested functions are not unwrapped
either.
+ */
+ @Nullable
+ private static String unwrapPureConversion(
+ String expression, Map<String, String> columnTypeNames) {
+ 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;
+ }
+ String typeName = columnTypeNames.get(inner);
+ boolean lossless;
+ if ("toDate".equalsIgnoreCase(matcher.group(1))) {
+ // toDate is lossless for Date (identity) and DateTime (drops
time-of-day, date part
+ // unchanged).
+ lossless = "Date".equalsIgnoreCase(typeName) ||
"DateTime".equalsIgnoreCase(typeName);
Review Comment:
Thanks for review. I've adopted the strict option: `toDate(col)` now unwraps
only when `col` is `Date` (same-type identity). `toDate(DateTime)` stays
unstructured with the raw expression preserved in `partition-key`, since the
effective timezone can shift the epoch boundary. `toDateTime` keeps its
existing branch. Done in commit `f66f7e8`.
--
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]
