fix: generate_series overflow panics at i64 boundary and out-of-range dates - #23723
fix: generate_series overflow panics at i64 boundary and out-of-range dates#23723u70b3 wants to merge 2 commits into
Conversation
00ccd1e to
bb38d8e
Compare
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23723 +/- ##
=======================================
Coverage 80.69% 80.69%
=======================================
Files 1095 1095
Lines 372623 372678 +55
Branches 372623 372678 +55
=======================================
+ Hits 300695 300740 +45
- Misses 53979 53981 +2
- Partials 17949 17957 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bb38d8e to
53ef65a
Compare
| .current | ||
| .should_stop(self.end.clone(), &self.step, false) | ||
| { | ||
| self.current.advance(&mut self.end, &self.step)?; |
There was a problem hiding this comment.
It looks like the generator still advances after emitting the inclusive end value. For date and timestamp series near the supported upper bound, this can turn a valid one-row query into an error.
For example:
SELECT * FROM generate_series(
DATE '2262-04-11',
DATE '2262-04-11',
INTERVAL '1' DAY
);This should return the single reachable row, but it currently fails with Failed to add interval ... to timestamp 9223286400000000000.
Could we apply the same terminal-state rule used for integer overflow and avoid calling advance once the current value is the last value to emit? One option would be to have advance report whether a next value exists. Another would be to break before advancing when the exclusive stop condition has already been reached.
| ---- | ||
| 9223372036854775806 | ||
|
|
||
| # Regression test for https://github.com/apache/datafusion/issues/22193 |
There was a problem hiding this comment.
It would be helpful to add positive boundary tests for the maximum representable date and timestamp, alongside the existing out-of-range error cases. That would lock in the expected behaviour of returning all reachable values at the edge of the nanosecond range.
|
Thanks for the review, @kosiew! Fixed the terminal boundary handling so |
… dates Two overflow fixes in the generate_series/range table functions: - Integer series: advancing past i64::MAX (or i64::MIN for negative steps) panicked in debug builds and silently wrapped in release. advance() now uses checked_add and clamps the series end so iteration stops after the last reachable value, matching PostgreSQL/DuckDB (e.g. generate_series(9223372036854775806, 9223372036854775807, 2) returns one row 9223372036854775806). - Date series: converting Date32 days to timestamp nanoseconds used an unchecked multiplication, panicking at planning time for dates outside the nanosecond timestamp range. It now returns a planning error. Supersedes apache#22250 (closed unmerged); the integer-side approach follows the one approved there. Closes apache#22208, closes apache#22193.
1ad77f4 to
eb22e6a
Compare
Which issue does this PR close?
Supersedes #22250 (closed unmerged by its author on 2026-06-04 after the approach was approved). Note: both issues are currently assigned to @xiedeyantu (idle since 2026-05; #22193 was explicitly blocked on #22250). I've left a note on both issues; happy to coordinate if the assignee has work in progress.
Rationale for this change
Two overflow bugs in the
generate_series/rangetable functions, both panicking in debug builds and silently wrapping in release:SELECT * FROM generate_series(9223372036854775806, 9223372036854775807, 2)panics withattempt to add with overflowwhen advancing pasti64::MAX. PostgreSQL and DuckDB both return one row (9223372036854775806), stopping after the last reachable value.Date32days to timestamp nanoseconds uses an unchecked multiplication, sogenerate_series(DATE '0001-01-01', ...)panics at planning time. Dates outside the nanosecond timestamp range (1677-09-21 – 2262-04-11) cannot be represented and must error cleanly instead.What changes are included in this PR?
datafusion/functions-table/src/generate_series.rsSeriesValue::advancenow takesend: &mut Self; thei64implementation useschecked_addand, on overflow, clampsendso the series terminates after the last reachable value (the approach approved in fix generate_series table function overflows #22250). The per-batch loop stops right after emitting the final value.TimestampValue::advancealready errors on out-of-range interval addition and is unchanged apart from the signature.checked_muland returns a planning error naming the offending argument.datafusion/sqllogictest/test_files/table_functions.slti64::MAX,range(end-exclusive) overflow, and first/second date argument out of range.Are these changes tested?
Yes:
cargo fmt, workspace clippy (avro,integration-tests,extended_tests,-D warnings),cargo test -p datafusion-functions-table, and the extended workspace suite (ci profile withavro,json,backtrace,extended_tests,recursive_protection,parquet_encryption) — all green.Are there any user-facing changes?
Only the bug fixes: integer series near
i64::MAX/MINnow return the reachable values instead of panicking or wrapping (matching PostgreSQL/DuckDB), and out-of-range dates produce a planning error instead of a panic. No API changes.