Skip to content

Commit 54a4bbf

Browse files
fix: do not panic when displaying a Date64 scalar of i64::MIN (#26139)
## Which issue does this PR close? - Closes #24892. ## 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. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 03204c0 commit 54a4bbf

1 file changed

Lines changed: 7 additions & 1 deletion

File tree

  • datafusion/common/src/scalar

‎datafusion/common/src/scalar/mod.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5646,7 +5646,10 @@ impl fmt::Display for ScalarValue {
56465646
f,
56475647
e.map(|v| {
56485648
let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
5649-
match epoch.checked_add_signed(Duration::try_milliseconds(v).unwrap())
5649+
// `try_milliseconds` is `None` for `i64::MIN`, which
5650+
// `Duration` cannot hold: treat it as out of range too
5651+
match Duration::try_milliseconds(v)
5652+
.and_then(|d| epoch.checked_add_signed(d))
56505653
{
56515654
Some(date) => date.to_string(),
56525655
None => "".to_string(),
@@ -10127,6 +10130,9 @@ mod tests {
1012710130
format!("{}", ScalarValue::Date64(Some(-790179464505600000))),
1012810131
""
1012910132
);
10133+
// These used to panic too, see https://github.com/apache/datafusion/issues/24892
10134+
assert_eq!(format!("{}", ScalarValue::Date64(Some(i64::MIN))), "");
10135+
assert_eq!(format!("{}", ScalarValue::Date64(Some(i64::MAX))), "");
1013010136
}
1013110137

1013210138
#[test]

0 commit comments

Comments
 (0)