Repository navigation
fix(array): accept timestamps beyond jiff's range - #10953
Open
gitRasheed wants to merge 4 commits into
Open
gitRasheed wants to merge 4 commits into
gitRasheed wants to merge 4 commits into
Conversation
Timestamp scalars were validated against `jiff::Timestamp`'s range, which stops 26 hours short of the end of year 9999, so a value Arrow, Parquet and Spark allow, such as `9999-12-31T00:00:00Z`, could neither be used as a literal nor written: the file statistics built the min/max scalar with `Scalar::extension_ref` and panicked. Every `i64` count of the unit is a valid timestamp; jiff only bounds what `Display` can render, so drop the range check, fall back to the raw count when the calendar rendering is unavailable, and build the min/max scalars fallibly so a rejected extreme is an error rather than a panic. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
Now that timestamps beyond jiff's range can be written, `split_temporal` could see second and millisecond counts more than i32::MAX days from the epoch. `split_with_divisor` truncated the days to i32 with `as_()`, so such values silently wrapped when compressed: `i64::MAX` seconds round-tripped as 50027779063807. The compress comment still claimed jiff validation made the narrowing safe, the btrblocks TemporalScheme applied the encoding with no range check, and `try_extract_days_constant` used the same truncating split, so `dtp <= lit` with such a literal was rewritten to a wrapped days constant. Microsecond and nanosecond counts cannot overflow the days part. `timestamp::split` now errors when the days do not fit, so the days pushdown declines the constant and the compare kernel falls back to the canonical comparison. `split_temporal` checks the valid min/max of second and millisecond arrays and errors instead of wrapping, and the new `can_split_temporal` lets the btrblocks TemporalScheme skip such arrays so the storage array is compressed and the data round-trips exactly. The narrowing compare test built its fixture from `i64::MAX` seconds and relied on the wrap; it now uses a value above i32::MAX whose days fit. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
…hold the literal Compression narrows the days part, for example to u16 for dates before 2149. A midnight literal past that range, such as 9999-12-31, made the comparison push-down cast its days to the narrowed type and fail with "Cannot cast i32 to u16". The rule now declines, and the comparison runs on the full timestamps. Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
Signed-off-by: gitRasheed <99349713+gitRasheed@users.noreply.github.com>
This branch has not been deployed
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.
Summary
Fixes #8860
Timestamp scalars were range-checked against
jiff::Timestamp, which ends 26 hours before the end of year 9999, so writing9999-12-31, a common "no end date" value, panicked while building file statistics. As discussed on #8860, these values are accepted rather than rejected. This picks up #9361, which went stale.Changes
Timestampno longer checks the stored value against jiff's range, andunpack_nativeno longer builds ajiff::Span, which also panicked for large values.Displayprints the raw count, such as253402214400000000µs, when jiff cannot render it.Scalar::try_new, so a value an extension type rejects is an error instead of a panic.i32::MAXdays from the epoch, and DateTimeParts wrapped the days part.split_temporalnow errors for such arrays, the btrblocksTemporalSchemeskips them, and the days-constant pushdown declines such literals.ts = '9999-12-31'hit it.Checked locally:
cargo test -p vortex-array --lib -- timestamp min_max extension(205 passed),-p vortex-datetime-parts(101),-p vortex-btrblocks(91), clippy with-D warnings, nightly fmt.I used AI tools to help write and review this change.