Skip to content

fix: do not panic when displaying a Date64 scalar of i64::MIN - #26139

Merged
neilconway merged 1 commit into
apache:mainfrom
geographybuff:24892-date64-display-i64-min
Oct 8, 2026
Merged

neilconway merged 1 commit into
apache:mainfrom
geographybuff:24892-date64-display-i64-min

Conversation

@geographybuff

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

impl fmt::Display for ScalarValue unwraps chrono::Duration::try_milliseconds(v) in the Date64 arm. That returns None for i64::MIN, which Duration cannot represent, so printing such a literal panics, for example EXPLAIN SELECT arrow_cast(-9223372036854775808, 'Date64').

#24893 proposed the same change and was closed as part of a batch of PRs, not on its merits. This PR redoes it on current main.

What changes are included in this PR?

try_milliseconds and checked_add_signed are chained with and_then, so a value Duration cannot hold falls into the existing None => "" branch. That's how the neighbouring out-of-range case already formats (added for apache/arrow-rs#7728). One file, +7/-1.

What is the testing strategy for this PR?

test_display_date64_large_values now also checks i64::MIN and i64::MAX:

  • Without the fix it panics at the same unwrap (scalar/mod.rs:5651).
  • With the fix, cargo test -p datafusion-common --lib passes 623 tests, and rustfmt --check is clean.

cargo clippy -p datafusion-common --all-targets -- -D warnings fails locally on an unused import in config.rs:4157. That happens on unmodified main too, probably because I ran without the parquet feature, so it's unrelated.

Not verified: the issue's second reproduction, a RANGE window frame over a Date64 column containing i64::MIN. I only ran the unit test. If that path panics somewhere other than this Display impl, this PR doesn't fix it.

Are there any user-facing changes?

No panic: a Date64 of i64::MIN displays as an empty string, like other out-of-range dates.

AI disclosure: prepared with Claude Code (Claude Opus 5.5), which wrote the change and the test and ran the commands above.

`Duration::try_milliseconds` returns `None` for `i64::MIN`, and the
Date64 arm of `impl Display for ScalarValue` unwrapped it. Chain it with
`checked_add_signed` so a value `Duration` cannot hold formats as an
empty string, like an out-of-range date already does.

Closes apache#24892

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the common Related to common crate label Oct 8, 2026
@geographybuff
geographybuff marked this pull request as ready for review October 8, 2026 13:56

@neilconway neilconway 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.

Thanks @geographybuff !

@neilconway
neilconway enabled auto-merge October 8, 2026 15:09
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.74%. Comparing base (3ed377a) to head (dc193ac).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26139      +/-   ##
==========================================
- Coverage   82.74%   82.74%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      449767   449770       +3     
  Branches   449767   449770       +3     
==========================================
- Hits       372160   372152       -8     
- Misses      54938    54946       +8     
- Partials    22669    22672       +3     

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

@neilconway
neilconway added this pull request to the merge queue Oct 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Oct 8, 2026
@neilconway
neilconway added this pull request to the merge queue Oct 8, 2026
Merged via the queue into apache:main with commit 54a4bbf Oct 8, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Displaying a Date64 scalar of i64::MIN panics

3 participants