Repository navigation
fix(datetime): error on out-of-range date_bin timestamp bins - #26607
Merged
Merged
Conversation
benbellick
commented
Oct 22, 2026
| } | ||
|
|
||
| #[test] | ||
| fn test_date_bin_preserves_timestamp_nullness() { |
Contributor
Author
There was a problem hiding this comment.
mirrors the date_trunc nullness-preservation tests
datafusion/datafusion/functions/src/datetime/date_trunc.rs
Lines 1017 to 1063 in c7a7f94
benbellick
commented
Oct 22, 2026
| } | ||
|
|
||
| #[test] | ||
| fn test_date_bin_timestamp_scalar_and_array_agree() { |
Contributor
Author
There was a problem hiding this comment.
related date_trunc scalar/array consistency test
datafusion/datafusion/functions/src/datetime/date_trunc.rs
Lines 1930 to 1991 in 2717e25
Contributor
Author
|
cc @jayzhan211 since you reviewed the last PR related to a similar fix for |
jayzhan211
approved these changes
Oct 23, 2026
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Thanks @benbellick ! LGTM
This was referenced Oct 23, 2026
jayzhan211
pushed a commit
to CuteChuanChuan/datafusion
that referenced
this pull request
Oct 31, 2026
…he#26835) ## Which issue does this PR close? Closes apache#25344. Closes apache#25345. This completes the initial single-key work alongside apache#26449. Explicit origins (apache#26913) and compound range keys (apache#24644) remain follow-ups. ## Rationale for this change Extend the aggregate-locality optimization from apache#26449 to `date_bin`. When bins stay within timestamp range partitions, aggregation can skip the hash shuffle. ## What changes are included in this PR? Pass literal argument values to the existing admission hook. `INTERVAL '1 hour'` and `INTERVAL '1 month'` have the same `IntervalMonthDayNano` type, so types alone cannot distinguish them. Inspecting the literal value lets `date_bin` require a positive fixed-duration stride without months, treating each day as 24 hours. `date_trunc` continues to rely only on argument types. Initially, admit only two-argument calls over timezone-less nanosecond timestamps, using that literal stride. Explicit origins and other types remain excluded. For example: ```sql SELECT date_bin(INTERVAL '1 hour', event_time) AS hour_bin, COUNT(*) FROM events GROUP BY hour_bin; ``` For ascending timestamp ranges: - A split at `01:00` separates hourly bins, so the shuffle can be skipped. - A split at `01:30` cuts through the `01:00` bin, so the shuffle remains. Every split must pass the existing boundary proof. Evaluation errors conservatively retain the shuffle. This follows the NULL-to-error fix in apache#26607. ## What is the testing strategy for this PR? Unit tests cover admission, hook forwarding, ascending/descending boundaries, and evaluation errors. SQLLogicTests verify plans and results for aligned bins, crossing bins, and excluded explicit origins. ## Are there any user-facing changes? Eligible `date_bin` aggregations can avoid a hash shuffle. Function results are unchanged. --- Note: This PR was developed with AI assistance. All changes have been reviewed, and I take full responsibility for this contribution.
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
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?
Closes #26605.
Rationale for this change
date_binreturns NULL for unrepresentable timestamp bins. Return an error instead, matching PostgreSQL.I'm not sure whether DataFusion has a specific policy on returning NULL versus an error here, but the PostgreSQL precedent seems clear. Returning NULL because we cannot compute a bin for a non-null timestamp silently hides a computation failure rather than representing missing input.
This prepares
date_binfor avoiding aggregate repartitioning in #25345, similar to thedate_truncoptimization in #26449. It follows the error behavior established fordate_truncin #25131.What changes are included in this PR?
Make scalar and array timestamp-binning failures return
DATE_BIN timestamp out of range. Preserve NULL inputs and successful wider-arithmetic results.What is the testing strategy for this PR?
Unit tests and SQLLogicTests cover overflow errors, scalar/array consistency, NULL preservation, and representable results across timestamp precisions.
Are there any user-facing changes?
Previously NULL-producing timestamp-binning failures now fail the query.
This PR was developed with AI assistance. All changes have been reviewed, and I take full responsibility for this contribution.