jiangxt2 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4450672172
##########
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 the update. I reviewed commit
[`3df1577`](https://github.com/apache/gravitino/commit/3df15772a472ce20e38312599437a70008615b10):
the native `TYPE_NAME` and conversion-direction checks now keep `DateTime64`
narrowing conversions unstructured, with the original `partition-key`
preserved. The added unit test and load-path IT cover the reported case. This
addresses the issue raised here.
--
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]
