fix: unwrap_cast_in_comparison drops the timezone shift when unwrapping CAST(timestamp AS timestamptz) = literal by Ruchirtripathi · Pull Request #25099 · apache/datafusion

Ruchirtripathi · 2026-09-09T05:53:50Z

Which issue does this PR close?

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. This shift effectively changes the underlying integer value by the local timezone offset.

Because is_lossy_temporal_cast did not recognize this as a lossy operation, the optimizer incorrectly stripped the cast and copied the underlying literal's UTC integer without applying the required timezone shift. As a result, the query returned rows offset by exactly the session timezone 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.
- UTC has a zero offset, so the underlying integer remains unchanged and the cast can safely be unwrapped.
- Non-UTC timezones require a shift, so the cast must be preserved.
This prevents unwrap_cast_in_comparison from stripping timezone shifts from the execution layer, allowing the physical layer to correctly handle the timezone conversion through Arrow's compute kernels.
Updated the sqllogictest in datafusion/sqllogictest/test_files/datetime/timestamps.slt, which was previously asserting the incorrect empty result 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:
- Conversions involving UTC are considered lossless.
- Conversions involving non-UTC timezones are correctly considered lossy.
Adjusted the expectations in datafusion/sqllogictest/test_files/datetime/timestamps.slt to reflect the correct behavior.

Are there any user-facing changes?

Yes. This is a 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 the behavior of PostgreSQL and DuckDB.

…n timezone matching is lossy This prevents the optimizer from dropping timezone shifts when casting between a timezone-aware and timezone-naive timestamp in comparisons.

codecov-commenter · 2026-09-09T08:32:55Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.80%. Comparing base (574fe67) to head (b84ba80).
⚠️ Report is 328 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25099      +/-   ##
==========================================
+ Coverage   81.17%   81.80%   +0.62%     
==========================================
  Files        1109     1130      +21     
  Lines      388164   417733   +29569     
  Branches   388164   417733   +29569     
==========================================
+ Hits       315109   341712   +26603     
- Misses      54509    55887    +1378     
- Partials    18546    20134    +1588

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Fixes incorrect results when the optimizer unwraps casts between timezone-naive and timezone-aware timestamps, which can drop required timezone shifts for non-UTC contexts.

Changes:

Update is_lossy_temporal_cast to treat naive ↔ tz-aware timestamp casts as lossy for non-UTC timezones.
Add unit coverage for the new lossy-cast behavior around timestamp timezones.
Update an sqllogictest expectation for a timestamp comparison involving AT TIME ZONE.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
datafusion/expr-common/src/casts.rs	Adds lossy detection for naive ↔ tz-aware timestamp casts (non-UTC) and a unit test for the behavior.
datafusion/sqllogictest/test_files/datetime/timestamps.slt	Adjusts expected output for a timezone-related timestamp comparison query.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    if let (DataType::Timestamp(_, from_tz), DataType::Timestamp(_, to_tz)) =
+        (from_type, to_type)
+        && from_tz.is_some() != to_tz.is_some()
+    {
+        let tz = from_tz.as_ref().or(to_tz.as_ref()).unwrap().as_ref();
+        if tz != "UTC"
+            && tz != "+00:00"
+            && tz != "-00:00"
+            && tz != "+0:00"
+            && tz != "-0:00"
+            && tz != "Z"
+        {
+            return true;
+        }
+    }


 SELECT column1 FROM t_europe WHERE column1 = '2024-01-31T16:00:01' AT TIME ZONE 'America/Los_Angeles';
 ----
-2024-02-01T00:00:01+01:00



+    #[test]
+    fn test_is_lossy_temporal_cast_timestamp_tz() {
+        let ts_naive = DataType::Timestamp(TimeUnit::Millisecond, None);
+        let ts_utc = DataType::Timestamp(TimeUnit::Millisecond, Some("UTC".into()));
+        let ts_sgt =
+            DataType::Timestamp(TimeUnit::Millisecond, Some("Asia/Singapore".into()));
+
+        // Naive <-> UTC is NOT lossy (UTC offset is 0, so literal cast is exact)
+        assert!(!is_lossy_temporal_cast(&ts_naive, &ts_utc));
+        assert!(!is_lossy_temporal_cast(&ts_utc, &ts_naive));
+
+        // Naive <-> Non-UTC is lossy because it ignores session timezone
+        assert!(is_lossy_temporal_cast(&ts_naive, &ts_sgt));
+        assert!(is_lossy_temporal_cast(&ts_sgt, &ts_naive));
+
+        // Tz-aware <-> Tz-aware is not lossy (both are UTC under the hood)
+        assert!(!is_lossy_temporal_cast(&ts_utc, &ts_sgt));
+        assert!(!is_lossy_temporal_cast(&ts_sgt, &ts_utc));
+    }


adriangb

Thank you for the fix. The approach is correct, and I verified that it corrects the bug. I have some requests before we merge.

adriangb · 2026-09-09T13:02:11Z

+        if tz != "UTC"
+            && tz != "+00:00"
+            && tz != "-00:00"
+            && tz != "+0:00"
+            && tz != "-0:00"
+            && tz != "Z"
+        {


This list is not exhaustive. Etc/UTC and GMT have an offset of zero, but the code does not accept them, and thus the optimizer keeps the cast:

-- "UTC": the optimizer removes the cast EXPLAIN SELECT * FROM t WHERE arrow_cast(ts, 'Timestamp(Nanosecond, Some("UTC"))') = arrow_cast(TIMESTAMP '2024-11-01T00:00:00', 'Timestamp(Nanosecond, Some("UTC"))'); -- predicate: ts = 1730419200000000000 -- "Etc/UTC": the optimizer keeps the cast, although the offset is also zero EXPLAIN SELECT * FROM t WHERE arrow_cast(ts, 'Timestamp(Nanosecond, Some("Etc/UTC"))') = arrow_cast(TIMESTAMP '2024-11-01T00:00:00', 'Timestamp(Nanosecond, Some("Etc/UTC"))'); -- predicate: CAST(ts AS Timestamp(Nanosecond, Some("Etc/UTC"))) = ...

GMT gives the same result as Etc/UTC. The direction of the error is safe, thus the rows stay correct. But the code loses the optimization for these names, and a list of strings is difficult to keep correct.

arrow::array::timezone::Tz parses all of these names. Please parse the timezone and test the offset instead of the list. If you keep the list, please move it into a function with a name such as is_zero_offset_timezone, and give the reason for each item.

adriangb · 2026-09-09T13:04:53Z

    if is_date_type(from_type) && is_date_type(to_type) {
        return false;
    }
+    if let (DataType::Timestamp(_, from_tz), DataType::Timestamp(_, to_tz)) =


I verified that this guard corrects the four queries in the issue, thank you. Four points on the block:

Please add the new rule to the doc comment of is_lossy_temporal_cast. The comment gives the rules for identity casts, date casts and Date32/Date64 casts in detail. It says nothing about timezones.

A cast between a naive timestamp and a timezone-aware timestamp is not lossy. The cast is bijective: it moves the value by the offset of the timezone. The doc comment of this function describes a different problem, which is a cast that is many-to-one. The guard gives the correct result, but the name and the comment now disagree with the code. Please make the reason clear at this position.

The issue gives a second solution: keep the unwrap, but move the literal by the same offset. That solution keeps the optimization. The present solution stops the unwrap, and thus the engine loses the pushdown and the pruning for each of these comparisons. Did you examine the second solution? If you prefer the present solution, please add a comment that says that the guard is intentionally conservative.

unwrap() is safe here, because is_some() != is_some() makes sure that one side has a value. But a match on the two options is more clear, and it removes the unwrap().

adriangb · 2026-09-09T13:05:06Z

    }

+    #[test]
+    fn test_is_lossy_temporal_cast_timestamp_tz() {


This test examines is_lossy_temporal_cast alone. It does not show that a query gives the correct rows. If a subsequent change makes unwrap_cast_in_comparison drop the timezone shift again, this test stays green, and the bug comes back without a failure.

Please add the queries from the issue to datafusion/sqllogictest/test_files/datetime/timestamps.slt. They are the only tests that show the behavior that this PR corrects:

statement ok set datafusion.execution.time_zone = 'Asia/Singapore'; statement ok create table t as select TIMESTAMP '2024-11-01T00:00:00' as ts; statement ok create table u as select '2024-10-31T16:00:00Z'::timestamptz as tstz; # 2024-11-01 00:00 in Singapore is 2024-10-31 16:00 UTC query I select count(*) from t where ts::timestamptz = '2024-10-31T16:00:00Z'::timestamptz; ---- 1 query I select count(*) from t where ts::timestamptz = '2024-11-01T00:00:00Z'::timestamptz; ---- 0 # the same rewrite occurs for an implicit coercion query I select count(*) from t where ts = '2024-10-31T16:00:00Z'::timestamptz; ---- 1 # control: a column against a column, thus the optimizer unwraps nothing query I select count(*) from t, u where t.ts::timestamptz = u.tstz; ---- 1

Please add two more cases:

A timezone-aware column against a timezone-naive literal. This is the opposite direction of the cast, and the guard is symmetric.

An explain for a UTC session timezone, which shows that the optimizer still removes the cast. Without this test, a subsequent guard that is too strong can remove the optimization for all timezones, and each test above stays green.

adriangb · 2026-09-09T13:05:21Z

 query P
 SELECT column1 FROM t_europe WHERE column1 = '2024-01-31T16:00:01' AT TIME ZONE 'America/Los_Angeles';
 ----
-2024-02-01T00:00:01+01:00


I verified this change on your branch. The empty result is correct, but the PR does not give the reason, and the description says the opposite (see the note from the Copilot review). Please put the reason in the PR description.

The arithmetic:

t_europe holds three instants: 2023-12-31T23:00:01Z, 2024-01-31T23:00:01Z and 2024-02-29T23:00:01Z. The union test below this one shows the same three instants.

The literal '2024-01-31T16:00:01' AT TIME ZONE 'America/Los_Angeles' is the instant 2024-02-01T00:00:01Z. The t_utc test below this one shows the same instant.

No row of t_europe is equal to that instant. Thus the empty result is correct, and the deleted row was a result of the bug.

An empty result is a weak assertion. A guard that is too strong also gives an empty result, and this test cannot see the difference. Please keep a row here. The instant 2024-01-31T23:00:01Z is 15:00:01 in Los Angeles, thus this query selects the second row:

query P SELECT column1 FROM t_europe WHERE column1 = '2024-01-31T15:00:01' AT TIME ZONE 'America/Los_Angeles'; ---- 2024-02-01T00:00:01+01:00

I ran this query two times on your branch: one time with your change, and one time with the new guard removed. Without the guard it gives zero rows, which is incorrect. With the guard it gives the row above. It is thus a correct test for this bug. Please keep the empty case also.

adriangb · 2026-09-09T17:29:30Z

+        "UTC" | "Etc/UTC" | "GMT" | "Etc/GMT" | "Greenwich" | "Z" => true,
+        // Common fixed offset zero strings parsed by Arrow
+        "+00:00" | "-00:00" | "+0:00" | "-0:00" => true,
+        _ => false,


Is there no function in arrow-rs or chrono we could use for this?

The arrow::array::timezone::Tz enum only allows fetching the offset dynamically for a specific NaiveDateTime via offset_from_utc_datetime(). However, we cannot simply instantiate an arbitrary date (like the Unix Epoch) and check if the offset is zero because geographic timezones like Europe/London evaluate to an offset of 0 during winter time. This would create false positives during query planning.

Because we need to evaluate this statically during optimization (where we don't want to instantiate values just to check offsets), explicitly whitelisting the known permanent zero-offset strings is currently the safest and most robust approach.

Ruchirtripathi added 6 commits September 9, 2026 10:27

Fix issue apache#25095: Reject unwrap_cast for timestamp literals whe…

6f6e5b4

…n timezone matching is lossy This prevents the optimizer from dropping timezone shifts when casting between a timezone-aware and timezone-naive timestamp in comparisons.

style: format code with cargo fmt

ab0676b

style: fix clippy collapsible_if warning

df079a8

fix: correct test expectations for UTC lossy casts

81b3579

test: fix broken timestamps sqllogictest expectations due to bugfix

156a6a2

style: fix trailing whitespace

d0ad665

github-actions Bot added logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels Sep 9, 2026

fix: treat +0:00 and -0:00 as UTC for lossless temporal cast check

b84ba80

adriangb requested a balanced review from Copilot September 9, 2026 12:41

Copilot AI reviewed Sep 9, 2026

View reviewed changes

adriangb requested changes Sep 9, 2026

View reviewed changes

kumarUjjawal changed the title ~~Fix issue #25095 unwrap_cast_in_comparison drops the timezone shift when unwrapping CAST(timestamp AS timestamptz) = literal~~ fix: unwrap_cast_in_comparison drops the timezone shift when unwrapping CAST(timestamp AS timestamptz) = literal Sep 9, 2026

fix: extract is_zero_offset_timezone and support Etc/UTC and GMT

e60c001

adriangb reviewed Sep 9, 2026

View reviewed changes

Copilot started reviewing on behalf of adriangb September 9, 2026 17:31 View session

Ruchirtripathi added 2 commits September 9, 2026 23:05

fix: test timezone-aware cast unwrap and refine comments

7a9760c

test: restore valid row selection for t_europe timezone query

2cea9a2

Navigation Menu

Uh oh!

fix: unwrap_cast_in_comparison drops the timezone shift when unwrapping CAST(timestamp AS timestamptz) = literal - #25099

fix: unwrap_cast_in_comparison drops the timezone shift when unwrapping CAST(timestamp AS timestamptz) = literal#25099
Ruchirtripathi wants to merge 10 commits into
apache:mainfrom
Ruchirtripathi:fix-issue-25095

Ruchirtripathi commented Sep 9, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

adriangb left a comment

Uh oh!

adriangb Sep 9, 2026

Uh oh!

adriangb Sep 9, 2026

Uh oh!

adriangb Sep 9, 2026

Uh oh!

adriangb Sep 9, 2026

Uh oh!

adriangb Sep 9, 2026

Uh oh!

Ruchirtripathi Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Ruchirtripathi commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Sep 9, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

adriangb left a comment

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Ruchirtripathi Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ruchirtripathi commented Sep 9, 2026 •

edited

Loading