jiangxt2 commented on code in PR #13749:
URL: https://github.com/apache/gravitino/pull/13749#discussion_r4450445046
##########
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:
**[P2] Guard year/month unwrapping at the timezone epoch boundary**
On ClickHouse 24.8.14.39 with default timezone `America/Los_Angeles`, a
`DateTime` column without an explicit timezone holding Unix timestamp `0`
returns `1970` for `toYear(toDate(ts))` but `1969` for `toYear(ts)`. Removing
`toDate` therefore exposes structured partition metadata that differs from the
original key.
Please use the column's effective timezone to decide whether `toDate` can be
removed; `serverTimezone()` or the reader's session timezone is insufficient.
If this cannot be established reliably, leave the `toYear`/`toYYYYMM`
expression unstructured and preserve `partition-key`. Keep the other conversion
branches unchanged. Please cover the cross-session case below, preserve unknown
offsets, and propagate lookup failures.
Concrete regression example: run these blocks in **two separate client
sessions against the same UTC server**.
**Session 1:**
```sql
SET session_timezone = 'America/Los_Angeles';
CREATE TABLE default.gravitino_epoch_boundary_review (ts DateTime) ENGINE =
Memory;
INSERT INTO default.gravitino_epoch_boundary_review
SELECT toDateTime(toUInt32(0));
```
**Session 2: reconnect with a new UTC client:**
```sql
SET session_timezone = 'UTC';
SELECT serverTimezone() AS server_tz,
timezoneOf(ts) AS column_tz,
toTypeName(ts) AS source_type,
toYear(toDate(ts)) AS original_year,
toYear(ts) AS simplified_year,
toYYYYMM(toDate(ts)) AS original_month,
toYYYYMM(ts) AS simplified_month
FROM default.gravitino_epoch_boundary_review;
-- Expected: UTC | America/Los_Angeles | DateTime | 1970 | 1969 | 197001 |
196912
DROP TABLE default.gravitino_epoch_boundary_review;
```
--
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]
