Ruchirtripathi opened a new pull request, #25099:
URL: https://github.com/apache/datafusion/pull/25099
## Which issue does this PR close?
- Closes #25095
## Rationale for this change
Comparing a timezone-naive timestamp column against a timezone-aware
literal (or vice-versa) was incorrectly
returning the wrong rows when the session timezone was not UTC.
The optimizer's `unwrap_cast_in_comparison` rule was rewriting `CAST(ts
AS timestamptz) = <literal>` into `ts =
CAST(<literal> AS timestamp_naive)`. However, the lower-level function
governing this unwrap
(`is_lossy_temporal_cast`) failed to recognize that casting between
timezone-naive and timezone-aware timestamps
acts as a timezone shift, effectively shifting the underlying integer by
the local offset. Because it didn't
recognize this as a lossy operation, the optimizer erroneously stripped
the cast and copied the underlying literal's
UTC integer without shifting it, causing the query to be offset by exactly
the session offset.
## What changes are included in this PR?
- Updated `is_lossy_temporal_cast` in
`datafusion/expr-common/src/casts.rs` to treat casts between timezone-naive
and timezone-aware timestamps as lossy operations *unless* the timezone is
UTC (since UTC has a zero offset, meaning
the literal's underlying integer remains completely identical and can be
safely unwrapped).
- This prevents `unwrap_cast_in_comparison` from stripping timezone
shifts from the execution layer, allowing the
physical layer to correctly handle the shift via Arrow's compute kernels.
- Updated the `sqllogictest` in `timestamps.slt` which was previously
asserting the buggy empty output for
`column1 = '2024-01-31T16:00:01' AT TIME ZONE 'America/Los_Angeles'`.
## What is the testing strategy for this PR?
- Added a new unit test `test_is_lossy_temporal_cast_timestamp_tz` in
`datafusion/expr-common/src/casts.rs` to
explicitly verify that UTC conversions are considered lossless while
conversions with other timezones correctly
return `true` (lossy).
- Adjusted expectations in
`datafusion/sqllogictest/test_files/datetime/timestamps.slt` which were
previously
relying on the broken behavior.
## Are there any user-facing changes?
Yes, bug fix: queries comparing a timezone-naive timestamp column
against a `timestamptz` literal will now return
the correct rows according to the session timezone, matching Postgres and
DuckDB behavior.
--
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]
