Skip to content

Commit c0e872f

Browse files
viiryaalamb
andauthored
fix: bin coarse date_bin timestamps in i128 and restore ordering (#25815)
## Which issue does this PR close? - Closes #25812. ## Rationale for this change #25692 stopped `date_bin` from propagating the ordering of second, millisecond and microsecond timestamps (and of sources whose range type is unknown), because scaling them to i64 nanoseconds can overflow and the overflow became a per-row `NULL`. As discussed on that PR, this forces a full re-sort for `ORDER BY date_bin(..., time)` and prevents streaming aggregation for `GROUP BY date_bin(..., time)` over sorted input, for data that never gets near the overflow range. ## What changes are included in this PR? - The existing i64 nanosecond computation stays as the fast path. When it overflows (scaling, `source - origin`, or the bin arithmetic), the bin is recomputed in i128 in a `#[cold]` path, for both fixed-duration and month strides. - As a result, `Timestamp(Second)` values in 1653 or 2286 now bin to real values instead of `NULL`. A non-null input can still become `NULL` in two cases: - The correct bin is outside the range of the output type, e.g. a bin starting before the minimum value of the type. There is no correct value to return here. - A month stride is applied to a value outside the range of `DateTime<Utc>` (about ±262,000 years), which the month arithmetic goes through. Like the original i64 nanosecond overflow, this is a limit of the intermediate representation rather than of the output type, only much narrower. #25855 tracks fixing it by doing the month arithmetic without `DateTime<Utc>`. - `date_bin` propagates the ordering of its source for every precision again, reverting the guard from #25692. The remaining `NULL` cases above only affect extreme values. - Except for negative month strides: `bin_months` can move a bin past its source for them, so their output is not monotonic, even for ordinary dates. `date_bin` no longer propagates ordering for them. This was already wrong for every precision before #25692, and is still wrong for `Timestamp(Nanosecond)` on `main`. #25856 tracks defining the semantics of negative strides. ## What is the testing strategy for this PR? - Unit tests check the results for s/ms/us values outside the nanosecond range, month strides, `source - origin` overflow for nanoseconds, and the cases that still return `NULL`, in both the scalar and array paths. - `timestamps.slt`: the #25690 queries now return `1653-02-10T06:13:20`, `1970-01-01T00:00:00`, `2286-11-20T17:46:40` in order, and a new `EXPLAIN` shows the final sort is removed again. - `timestamps.slt`: negative month strides keep the final sort and return correctly ordered results for second and nanosecond timestamps, while a positive month stride still removes it. A unit test covers `output_ordering` for each kind of stride. - `date_bin_errors.slt`: three overflow cases that returned `NULL` now return their correct bins. - The `date_bin_1000` benchmark (`Timestamp(Second)`, in range) goes from 2.78 µs on `main` to 2.50 µs. ## Are there any user-facing changes? Coarse-precision `date_bin` results outside the nanosecond range are now correct values instead of `NULL`, and queries over them no longer add sorts. There are no public API changes. --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
1 parent b40d696 commit c0e872f

3 files changed

Lines changed: 456 additions & 117 deletions

File tree

0 commit comments

Comments
 (0)