cwq222 commented on PR #13749: URL: https://github.com/apache/gravitino/pull/13749#issuecomment-6574201175
@diqiu50 Thanks for the review. These are all good catches — here's how I addressed each one: 1. Agreed, and fixed. The `is_in_partition_key` fallback is removed entirely, so `toYear(toString(ts))` and `toDate(toStartOfMonth(x))` no longer unwrap — they now return an empty `partitioning()` with the raw expression in `partition-key`. The guard cases are now exercised through the two-arg overload with real column types in `testNonEquivalentExpressionsRemainUnstructured`. 2. Removed. The lossy assertion `toYYYYMM(toDate(statis_ymd + 1))` -> `month(statis_ymd)` is gone; `testNonEquivalentExpressionsRemainUnstructured` now asserts that the offset stays unstructured instead. 3. Done as suggested. `unwrapPureConversion` only unwraps `toDate(col)` / `toDateTime(col)` when `col` is a simple column whose source type is Date or Timestamp; everything else keeps an empty `partitioning()`. Added a round-trip test (`testPureTypeConversionRoundTrips`) and an IT (`CatalogClickHouseIT.testLoadTableWithNestedTypeConversion`). 4. Noted — `getTablePartitioning` is left as the single-arg overload since ClickHouse overrides `load()` and that path isn't reached. -- 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]
