alamb opened a new issue, #26289: URL: https://github.com/apache/datafusion/issues/26289
### Is your feature request related to a problem or challenge? Comparing a `Date32` / `Date64` column against a timestamp literal leaves a per-row `CAST(col AS Timestamp)` in the plan, even when the comparison could be rewritten exactly as a comparison on the raw date column. This is the optimization that [`unwrap_cast_in_comparison`](https://github.com/apache/datafusion/blob/c1180022e63f48a3af175d8d73dc21cca9379c65/datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs#L18-L41) exists to perform. Moving the cast from the column means the predicate becomes `col <op> literal` which is special cased in many other places in the code, for example to prune using bloom filters, etc. ### Reproducer Run with `datafusion-cli`: ```sql create table d (d date, v int) as values (date '2024-05-01', 1), (date '2024-05-02', 2); ``` An integer widening cast IS unwrapped (note there is no `CAST` in the Filter) ```sql > explain format indent select * from d where cast(v as bigint) = 1; +---------------+-----------------------------------------------------+ | plan_type | plan | +---------------+-----------------------------------------------------+ | logical_plan | Filter: d.v = Int32(1) | | | TableScan: d projection=[d, v] | | physical_plan | FilterExec: v@1 = 1 | | | DataSourceExec: partitions=1, partition_sizes=[1] | | | | +---------------+-----------------------------------------------------+ 2 row(s) fetched. Elapsed 0.009 seconds. ``` However, a Date -> Timestamp cast is NOT unwrapped (note the `CAST` on the timestamp column) ```sql > explain format indent select * from d where cast(d as timestamp) = timestamp '2024-05-01 00:00:00'; +---------------+-------------------------------------------------------------------------------------+ | plan_type | plan | +---------------+-------------------------------------------------------------------------------------+ | logical_plan | Filter: CAST(d.d AS Timestamp(ns)) = TimestampNanosecond(1714521600000000000, None) | <---- Note the CAST here | | TableScan: d projection=[d, v] | | physical_plan | FilterExec: CAST(d@0 AS Timestamp(ns)) = 1714521600000000000 | | | DataSourceExec: partitions=1, partition_sizes=[1] | | | | +---------------+-------------------------------------------------------------------------------------+ 2 row(s) fetched. Elapsed 0.003 seconds. ``` Similarly, implicit coercion adds casts CAST as well even if the user didn't write it explicitly ```sql > explain format indent select * from d where d >= timestamp '2024-05-02 00:00:00'; +---------------+--------------------------------------------------------------------------------------+ | plan_type | plan | +---------------+--------------------------------------------------------------------------------------+ | logical_plan | Filter: CAST(d.d AS Timestamp(ns)) >= TimestampNanosecond(1714608000000000000, None) | <---- Note the CAST here | | TableScan: d projection=[d, v] | | physical_plan | FilterExec: CAST(d@0 AS Timestamp(ns)) >= 1714608000000000000 | | | DataSourceExec: partitions=1, partition_sizes=[1] | | | | +---------------+--------------------------------------------------------------------------------------+ 2 row(s) fetched. Elapsed 0.002 seconds. ``` ### Expected The second and third plans should look like the `Date32` literal versions, specifically they should have no CAST function ``` Filter: d.d = Date32("2024-05-01") Filter: d.d >= Date32("2024-05-02") ``` ### Background `unwrap_cast_in_comparison` gained `Date32` / `Date64` support in this PR: - #21665 However, `is_lossy_temporal_cast` blocks converting in **both** directions between `Date` and `Timestamp`: https://github.com/apache/datafusion/blob/c1180022e63f48a3af175d8d73dc21cca9379c65/datafusion/expr-common/src/casts.rs#L85-L104 Blocking `Timestamp -> Date` is correct as that cast is many-to-one (every timestamp in a day maps to the same date), so `CAST(ts AS DATE) = DATE '2024-01-01'` cannot be rewritten to `ts = <single literal>`. See this for more details - #23095 Blocking `Date -> Timestamp`, on the other hand, is unnecessarily conservative: The cast is one-to-one on the column side as each date maps to exactly one timestamp (midnight), so there is always an exact rewrite. For example: | Original | Rewrite | |----------------------|-------------------------------------------------------| | `CAST(d AS ts) = L` | `d = date(L)` if `L` is at midnight, else no rewrite | | `CAST(d AS ts) != L` | `d != date(L)` if `L` is at midnight, else no rewrite | | `CAST(d AS ts) >= L` | `d >= ceil_to_day(L)` | | `CAST(d AS ts) > L` | `d > floor_to_day(L)` | | `CAST(d AS ts) <= L` | `d <= floor_to_day(L)` | | `CAST(d AS ts) < L` | `d < ceil_to_day(L)` | This came up while reviewing - #22114 ### Describe the solution you'd like Relax `is_lossy_temporal_cast` so that `Date -> Timestamp` (column cast direction) is allowed, using the literal rewrites described in the table above. Note timezone-aware timestamp literals (`Timestamp(_, Some(tz))`) need care: the `Date -> Timestamp(tz)` cast produces local midnight in `tz`, so the midnight check and the day rounding have to be done in the literal's zone. It is fine to restrict a first implementation to `Timestamp(_, None)`. ### Describe alternatives you've considered Adding a `Date -> Timestamp` to the code introduced in this issue may be enough - #22906 ### Additional context - #21665 added `Date32` / `Date64` support and the `is_lossy_temporal_cast` guard. - #23095 tracks the general safety of `unwrap_cast` for many-to-one casts. - #22114 (`date_trunc` preimage) works around this gap by peeling the cast. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
