Repository navigation
feat: include source field context in cast errors - #24920
Merged
Merged
Conversation
haohuaijin
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24920 +/- ##
=======================================
Coverage 81.59% 81.59%
=======================================
Files 1123 1123
Lines 410908 410939 +31
Branches 410908 410939 +31
=======================================
+ Hits 335263 335289 +26
- Misses 55901 55904 +3
- Partials 19744 19746 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adriangb
added a commit
to pydantic/datafusion
that referenced
this pull request
Sep 11, 2026
Upstream apache#24920 (merged 2026-09-06) adds field context to cast errors. The DST-boundary section pinned the full error chain, so 16 expectations started to fail once CI merged this PR onto current main -- 10 for America/Denver and 6 for Europe/Brussels: expected: DataFusion error: Arrow error: Cast error: Cannot cast timezone ... got: DataFusion error: Failed to cast field 'column1' from Timestamp(ns) to Timestamp(ns, "America/Denver") caused by Arrow error: Cast error: Cannot cast timezone to different timezone The wrapper chain is not what these cases pin. They pin which error class each path raises: the column path raises a Cast error, and the literal path raises a Parser error. So each expectation now matches only the `Arrow error: ...` tail. This uses the single-line `query error <regex>` form. In sqllogictest 0.29.1 an inline pattern goes through `Regex::is_match`, which is an unanchored search, while the multiline `----` form requires an exact match. The tail appears verbatim in both the old and the new message, so these expectations hold on either side of apache#24920 and will not break on the next wrapper change. Expectations and SQL are otherwise unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adriangb
added a commit
to pydantic/datafusion
that referenced
this pull request
Sep 15, 2026
Upstream apache#24920 (merged 2026-09-06) adds field context to cast errors. The DST-boundary section pinned the full error chain, so 16 expectations started to fail once CI merged this PR onto current main -- 10 for America/Denver and 6 for Europe/Brussels: expected: DataFusion error: Arrow error: Cast error: Cannot cast timezone ... got: DataFusion error: Failed to cast field 'column1' from Timestamp(ns) to Timestamp(ns, "America/Denver") caused by Arrow error: Cast error: Cannot cast timezone to different timezone The wrapper chain is not what these cases pin. They pin which error class each path raises: the column path raises a Cast error, and the literal path raises a Parser error. So each expectation now matches only the `Arrow error: ...` tail. This uses the single-line `query error <regex>` form. In sqllogictest 0.29.1 an inline pattern goes through `Regex::is_match`, which is an unanchored search, while the multiline `----` form requires an exact match. The tail appears verbatim in both the old and the new message, so these expectations hold on either side of apache#24920 and will not break on the next wrapper change. Expectations and SQL are otherwise unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adriangb
added a commit
to pydantic/datafusion
that referenced
this pull request
Sep 24, 2026
Upstream apache#24920 (merged 2026-09-06) adds field context to cast errors. The DST-boundary section pinned the full error chain, so 16 expectations started to fail once CI merged this PR onto current main -- 10 for America/Denver and 6 for Europe/Brussels: expected: DataFusion error: Arrow error: Cast error: Cannot cast timezone ... got: DataFusion error: Failed to cast field 'column1' from Timestamp(ns) to Timestamp(ns, "America/Denver") caused by Arrow error: Cast error: Cannot cast timezone to different timezone The wrapper chain is not what these cases pin. They pin which error class each path raises: the column path raises a Cast error, and the literal path raises a Parser error. So each expectation now matches only the `Arrow error: ...` tail. This uses the single-line `query error <regex>` form. In sqllogictest 0.29.1 an inline pattern goes through `Regex::is_match`, which is an unanchored search, while the multiline `----` form requires an exact match. The tail appears verbatim in both the old and the new message, so these expectations hold on either side of apache#24920 and will not break on the next wrapper change. Expectations and SQL are otherwise unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jay-dee7
pushed a commit
to o11y-one/datafusion
that referenced
this pull request
Sep 24, 2026
) ## Which issue does this PR close? None. This PR adds tests only. It records current behaviour, bugs included, for these issues: - apache#12218: `::timestamp` on a tz-aware value gives the UTC wall clock, and `tstz AT TIME ZONE` stays tz-aware - apache#12892: `from_unixtime` and the session time zone (fixed by apache#25161, pinned as fixed) - apache#13212: a tz-naive side of a comparison is read in the other operand's zone, not the session zone - apache#25084: casting a tz-naive value into a named zone errors at DST boundaries - apache#25095: unwrap_cast rewrites a tz-naive column compared with a tz-aware literal under a non-UTC session zone - apache#25166: `TIMESTAMP WITH TIME ZONE` / `::timestamptz` can resolve to a tz-naive type, and a cast to `::timestamptz` replaces an existing zone - apache#25167: `date_bin` and `date_trunc` disagree on tz-aware input - apache#25168: `date_bin` with an explicit origin drifts across a DST transition (PostgreSQL and DuckDB behave the same way) - apache#25170: `AT TIME ZONE '+05:30'` uses the opposite sign convention from PostgreSQL - apache#10368: `date_part('timezone_hour' | 'timezone_minute')` (implemented by apache#25163) ## Rationale for this change Timezone bugs keep coming back in DataFusion because the result depends on several things at once: the session time zone (set or unset), the value's own zone, DST, and optimizer rewrites such as unwrap_cast and constant folding. Few tests cover these combinations. Outside this file, the sqllogictest suite has 47 `SET datafusion.execution.time_zone` statements across 10 files. The PostgreSQL-compatibility tests had no timezone coverage until apache#25164. This file records what DataFusion does today for timestamps with a time zone, so any change to those semantics shows up as a diff in review instead of shipping silently. Where the recorded behaviour is wrong, or disagrees with PostgreSQL, a comment says so and links the issue. When a fix lands, the expected output changes and the comment goes away. apache#25164 is the companion PR. It checks against a real PostgreSQL the cases where the two engines agree. This file covers the cases where they don't. ## What changes are included in this PR? One new file (1674 lines), `datafusion/sqllogictest/test_files/datetime/timestamps_timezone.slt`, in 18 sections: | Section | What it pins | |---|---| | 0–4 | Type and value of `::timestamptz`, `TIMESTAMP [WITH TIME ZONE]` and table columns with the session zone unset, `+00:00`, `+05:30`, `America/Denver`, and `Europe/Brussels` | | 5 | `AT TIME ZONE` on tz-naive and tz-aware values, the composed `(tstz AT TIME ZONE z)::timestamp` idiom, and the `'+05:30'` sign convention | | 6 | Casts in every direction (naive/named/fixed offset), including `::timestamptz` on an already-zoned value | | 7 | Naive → aware → naive round trips, and the reverse | | 8 | Comparisons between tz-aware and tz-naive values, including the unwrap_cast case, with EXPLAIN of the rewritten filters | | 9 | `date_bin`, `date_trunc`, `date_part` (including `timezone*` fields) and `extract` on tz-aware input, including the explicit-origin DST case | | 10 | `to_char`, `from_unixtime`, `to_unixtime`, `to_timestamp*`, `to_local_time` | | 11 | `now`, `current_date`, `current_time`, `make_date` under a session zone | | 12 | `+`/`-` interval across DST, `'1 day'` vs `'24 hours'` | | 13–15 | Aggregates, GROUP BY, ORDER BY, DISTINCT, joins with mixed zones, and UNION / CASE / COALESCE / `greatest` type coercion | | 16 | Ambiguous and non-existent local times (US and EU transitions) | | 17 | A zone with no DST (America/Phoenix) and a half-hour zone (Asia/Kolkata) | No production code changes. ### Interaction with other open PRs Some fix PRs change expectations in this file. Whichever lands second updates the pins. I'd prefer the fixes land first so they stay small, and this file absorbs the change: - apache#25161 (`from_unixtime` follows the session zone) is already on main. The file pins the fixed behaviour. - apache#25163 (`date_part` timezone fields) is already on main. SECTION 9 pins `timezone`, `timezone_hour` and `timezone_minute` as values. - apache#25165 (`AT TIME ZONE` on a tz-aware value returns naive) changes the two SECTION 5b queries. - apache#25171 (docs) doesn't touch this file. Its wording that `'...Z'::timestamptz` "discards the `Z`" should match this file: the offset is applied, but the result carries no zone. - apache#25173 (`generate_series` precisions) doesn't touch this file. ## What is the testing strategy for this PR? The PR is the test. I generated expected output with sqllogictest `--complete`. Then I read every case and rewrote the ones that weren't testing what they meant to test, and added comments with issue links where the behaviour is wrong or differs from PostgreSQL. The file runs in DataFusion only. The PostgreSQL 15 answers quoted in comments come from separate runs, with the PostgreSQL `TimeZone` given in each comment. DuckDB 1.5.2 answers are quoted where they matter (the `date_bin` origin case, and the `'+05:30'` offset string). DST error expectations match only the stable `Arrow error: ...` tail, so changes to the wrapper error chain (for example apache#24920) don't break them. Run with: ``` cargo test -p datafusion-sqllogictest --test sqllogictests -- datetime/timestamps_timezone ``` ## Are there any user-facing changes? No. Tests only. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
diegoQuinas
pushed a commit
to diegoQuinas/datafusion
that referenced
this pull request
Sep 24, 2026
) ## Which issue does this PR close? None. This PR adds tests only. It records current behaviour, bugs included, for these issues: - apache#12218: `::timestamp` on a tz-aware value gives the UTC wall clock, and `tstz AT TIME ZONE` stays tz-aware - apache#12892: `from_unixtime` and the session time zone (fixed by apache#25161, pinned as fixed) - apache#13212: a tz-naive side of a comparison is read in the other operand's zone, not the session zone - apache#25084: casting a tz-naive value into a named zone errors at DST boundaries - apache#25095: unwrap_cast rewrites a tz-naive column compared with a tz-aware literal under a non-UTC session zone - apache#25166: `TIMESTAMP WITH TIME ZONE` / `::timestamptz` can resolve to a tz-naive type, and a cast to `::timestamptz` replaces an existing zone - apache#25167: `date_bin` and `date_trunc` disagree on tz-aware input - apache#25168: `date_bin` with an explicit origin drifts across a DST transition (PostgreSQL and DuckDB behave the same way) - apache#25170: `AT TIME ZONE '+05:30'` uses the opposite sign convention from PostgreSQL - apache#10368: `date_part('timezone_hour' | 'timezone_minute')` (implemented by apache#25163) ## Rationale for this change Timezone bugs keep coming back in DataFusion because the result depends on several things at once: the session time zone (set or unset), the value's own zone, DST, and optimizer rewrites such as unwrap_cast and constant folding. Few tests cover these combinations. Outside this file, the sqllogictest suite has 47 `SET datafusion.execution.time_zone` statements across 10 files. The PostgreSQL-compatibility tests had no timezone coverage until apache#25164. This file records what DataFusion does today for timestamps with a time zone, so any change to those semantics shows up as a diff in review instead of shipping silently. Where the recorded behaviour is wrong, or disagrees with PostgreSQL, a comment says so and links the issue. When a fix lands, the expected output changes and the comment goes away. apache#25164 is the companion PR. It checks against a real PostgreSQL the cases where the two engines agree. This file covers the cases where they don't. ## What changes are included in this PR? One new file (1674 lines), `datafusion/sqllogictest/test_files/datetime/timestamps_timezone.slt`, in 18 sections: | Section | What it pins | |---|---| | 0–4 | Type and value of `::timestamptz`, `TIMESTAMP [WITH TIME ZONE]` and table columns with the session zone unset, `+00:00`, `+05:30`, `America/Denver`, and `Europe/Brussels` | | 5 | `AT TIME ZONE` on tz-naive and tz-aware values, the composed `(tstz AT TIME ZONE z)::timestamp` idiom, and the `'+05:30'` sign convention | | 6 | Casts in every direction (naive/named/fixed offset), including `::timestamptz` on an already-zoned value | | 7 | Naive → aware → naive round trips, and the reverse | | 8 | Comparisons between tz-aware and tz-naive values, including the unwrap_cast case, with EXPLAIN of the rewritten filters | | 9 | `date_bin`, `date_trunc`, `date_part` (including `timezone*` fields) and `extract` on tz-aware input, including the explicit-origin DST case | | 10 | `to_char`, `from_unixtime`, `to_unixtime`, `to_timestamp*`, `to_local_time` | | 11 | `now`, `current_date`, `current_time`, `make_date` under a session zone | | 12 | `+`/`-` interval across DST, `'1 day'` vs `'24 hours'` | | 13–15 | Aggregates, GROUP BY, ORDER BY, DISTINCT, joins with mixed zones, and UNION / CASE / COALESCE / `greatest` type coercion | | 16 | Ambiguous and non-existent local times (US and EU transitions) | | 17 | A zone with no DST (America/Phoenix) and a half-hour zone (Asia/Kolkata) | No production code changes. ### Interaction with other open PRs Some fix PRs change expectations in this file. Whichever lands second updates the pins. I'd prefer the fixes land first so they stay small, and this file absorbs the change: - apache#25161 (`from_unixtime` follows the session zone) is already on main. The file pins the fixed behaviour. - apache#25163 (`date_part` timezone fields) is already on main. SECTION 9 pins `timezone`, `timezone_hour` and `timezone_minute` as values. - apache#25165 (`AT TIME ZONE` on a tz-aware value returns naive) changes the two SECTION 5b queries. - apache#25171 (docs) doesn't touch this file. Its wording that `'...Z'::timestamptz` "discards the `Z`" should match this file: the offset is applied, but the result carries no zone. - apache#25173 (`generate_series` precisions) doesn't touch this file. ## What is the testing strategy for this PR? The PR is the test. I generated expected output with sqllogictest `--complete`. Then I read every case and rewrote the ones that weren't testing what they meant to test, and added comments with issue links where the behaviour is wrong or differs from PostgreSQL. The file runs in DataFusion only. The PostgreSQL 15 answers quoted in comments come from separate runs, with the PostgreSQL `TimeZone` given in each comment. DuckDB 1.5.2 answers are quoted where they matter (the `date_bin` origin case, and the `'+05:30'` offset string). DST error expectations match only the stable `Arrow error: ...` tail, so changes to the wrapper error chain (for example apache#24920) don't break them. Run with: ``` cargo test -p datafusion-sqllogictest --test sqllogictests -- datetime/timestamps_timezone ``` ## Are there any user-facing changes? No. Tests only. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
Runtime cast errors currently identify the invalid value and target type but not the source field. This makes failures difficult to diagnose when a query contains multiple casts or an application processes many fields.
Including the source field in the error provides actionable context while retaining the original Arrow error as the cause.
What changes are included in this PR?
This PR adds context to errors returned by
CastExpr::evaluate:For example, the error now looks like:
Are these changes tested?
Yes. The existing invalid cast unit test now verifies the complete error, including the source field, source and target data types, and original Arrow error.
The following checks pass:
cargo fmt --all cargo test -p datafusion-physical-expr invalid_cast_with_options_error cargo clippy --all-targets --all-features -- -D warningsThe behavior was also verified end to end with the DataFusion CLI using the reproduction from #24919.
Are there any user-facing changes?
Yes. Runtime cast error messages now include the source field and source and target data types. The original cast error remains available as the cause.
There are no public API changes.