Skip to content

fix: annotate to_timestamp array results with the execution timezone - #24636

Open
adriangb wants to merge 1 commit into
apache:mainfrom
pydantic:claude/datafusion-issue-24632-4a017a
Open

fix: annotate to_timestamp array results with the execution timezone#24636
adriangb wants to merge 1 commit into
apache:mainfrom
pydantic:claude/datafusion-issue-24632-4a017a

Conversation

@adriangb

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

With datafusion.execution.time_zone set, to_timestamp on a string column declared a
return type of Timestamp(ns, <tz>) but produced an array of Timestamp(ns, None). The
declared type and the produced array disagreed, so any plan that materialized the column
failed at execution:

SET datafusion.execution.time_zone = 'UTC';
SELECT to_timestamp(s) AS v FROM (VALUES ('2026-01-01T00:00:00')) AS t(s);
-- Internal error: Function 'to_timestamp' returned value of type 'Timestamp(ns)' while
-- the following type was promised at planning time and expected: 'Timestamp(ns, "UTC")'

A second symptom: because the planner trusts the declared type it inserts no coercion, so a
comparison against another timezone-aware value reached the Arrow kernel with mismatched
types (Invalid comparison operation: Timestamp(ns) <= Timestamp(ns, "UTC")).

Scalar arguments were unaffected — they are constant-folded through a different code path
that did pick up the timezone — which is why every existing test passed.

What changes are included in this PR?

handle/handle_multiple in datafusion/functions/src/datetime/common.rs built the result
array straight from O::DATA_TYPE, which for timestamps carries no timezone, and ignored
the dt return type they were passed. The scalar branches of the same functions went
through scalar_value(dt, ..) and did carry the timezone — hence the mismatch.

The array branches now re-annotate the result with dt before returning it. to_date, the
other caller, returns Date32 and is unaffected.

Two existing unit tests in to_timestamp.rs asserted Timestamp(_, None) for array results
even though their helpers configure "UTC" as the execution timezone; those expectations
encoded the bug and are updated.

Are these changes tested?

Yes.

  • to_timestamp_timezone.slt gains coverage for non-constant-folded (array) input: the
    arrow_typeof and value for to_timestamp under UTC and America/New_York, the
    explicit-format overload, all four precision variants, the comparison from the issue, and
    a to_date case confirming it is unaffected. Every one of these fails on main.
  • A new to_timestamp_array_respects_execution_timezone unit test asserts, for each of the
    five to_timestamp* UDFs and both overloads, that the returned array's type equals what
    return_type advertises.

The full sqllogictest suite and the extended workspace test suite pass, as does
./dev/rust_lint.sh.

Are there any user-facing changes?

Yes, a bug fix: to_timestamp* on a string column now returns an array annotated with
datafusion.execution.time_zone, matching its declared return type, instead of failing at
execution. No API changes.

🤖 Generated with Claude Code

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Aug 24, 2026
@adriangb
adriangb marked this pull request as ready for review August 24, 2026 19:28
@adriangb
adriangb requested a balanced review from Copilot August 24, 2026 19:28
@adriangb

Copy link
Copy Markdown
Contributor Author

@kosiew @Omega359 would one of you be willing to review?

`handle`/`handle_multiple` built the result array straight from
`O::DATA_TYPE`, which for timestamps carries no timezone, while the
scalar branch went through `scalar_value(dt, ..)` and did pick up the
timezone from the declared return type.

With `datafusion.execution.time_zone` set, a string *column* argument
therefore produced `Timestamp(u, None)` while `return_type` promised
`Timestamp(u, <tz>)`, so any plan materializing the column failed, and
comparisons against another zoned value reached the Arrow kernel with
mismatched types.

Re-annotate the array with the declared return type before returning it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adriangb
adriangb force-pushed the claude/datafusion-issue-24632-4a017a branch from 96ca8ce to 09ca1ce Compare August 24, 2026 19:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes timezone metadata mismatches for array-based to_timestamp* results.

Changes:

  • Re-annotates parsed arrays with the declared return type.
  • Updates unit-test expectations and adds coverage across timestamp variants.
  • Adds SQL regression coverage for timezone-aware arrays and comparisons.

Reviewed changes

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

File Description
datafusion/functions/src/datetime/common.rs Applies declared timestamp timezone metadata to result arrays.
datafusion/functions/src/datetime/to_timestamp.rs Updates and expands unit tests.
datafusion/sqllogictest/test_files/to_timestamp_timezone.slt Adds end-to-end timezone regression tests.

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.12346% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.43%. Comparing base (26b40dd) to head (09ca1ce).

Files with missing lines Patch % Lines
datafusion/functions/src/datetime/common.rs 83.33% 3 Missing and 2 partials ⚠️
datafusion/functions/src/datetime/to_timestamp.rs 94.11% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24636      +/-   ##
==========================================
- Coverage   81.43%   81.43%   -0.01%     
==========================================
  Files        1118     1118              
  Lines      399414   399469      +55     
  Branches   399414   399469      +55     
==========================================
+ Hits       325278   325313      +35     
- Misses      55145    55157      +12     
- Partials    18991    18999       +8     

☔ 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.

@Omega359

Copy link
Copy Markdown
Contributor

I'll try and review this tonight @adriangb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

to_timestamp with string input returns an array whose timezone does not match its declared return type

4 participants