fix(query): correct date interval arithmetic and bounds (#20416) - #20425
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f68d8846
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
e23cf4a to
6295f2b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6295f2bfc4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
6295f2b to
d7d42bd
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7d42bd6dc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
3e163d9 to
a5d9226
Compare
…dlabs#20416) Replace Jiff-backed SQL datetime parsing, projection, formatting, and arithmetic with Chrono 0.4.45 and chrono-tz 0.10.4. Keep session time zones as `chrono_tz::Tz` and update FunctionContext, COPY, Variant, storage readers, format encoders, task history, and optimizer folding consumers accordingly. Support the complete SQL Date, Timestamp, and TIMESTAMP_TZ domain through `9999-12-31 23:59:59.999999`. Apply explicit offsets with checked arithmetic, reject results outside the SQL timestamp domain, and still render local year 10000 when a valid upper-bound instant is displayed through a positive offset. Unix-second formatted input is range checked as well. Use a per-day timezone lookup table for 1900..=2099 and fall back to chrono-tz outside that cache. Share one local-time resolver across parsing and arithmetic, including transitions at midnight and non-hour offset changes. Preserve the source side of a DST fold when it remains valid and use compatible gap resolution. Named zones after 2099 use chrono-tz's final recorded offset instead of Jiff's recurring-rule continuation. Correct interval behavior across timezone transitions. Calendar months and days operate on local wall-clock fields, elapsed microseconds operate on the instant timeline, and mixed intervals retain the established add/subtract ordering. Fixed-offset TIMESTAMP_TZ values remain DST-free, including `age` calculations. DATE +/- INTERVAL now returns TIMESTAMP, while existing date/timestamp clamp semantics are retained and unrepresentable large calendar deltas report errors. This also fixes upper-bound day/hour arithmetic and date conversion near year 9999. Move formatted timestamp parsing and rendering to Chrono's strftime dialect and make the resulting compatibility changes explicit: - omitted civil and clock fields are completed from 1970-01-01 00:00:00; - `%C` can supply a century by itself and renders without forced two-digit padding; - `%y` and `%g` use the 1969..=2068 two-digit-year window; - ISO week-year fields and AM/PM combinations are completed consistently; - bare `%f` parses one to nine fractional digits and renders nine nanosecond digits; - `%s` is accepted only as a standalone instant and is checked against the SQL timestamp range; - seconds 60 and 61 are rejected instead of being represented or constrained as leap seconds, while try variants return NULL; - `%Z` follows Chrono's numeric-offset rendering and `%+` is accepted; - Jiff extensions `%Q`, `%:Q`, and `%N` are no longer accepted; - `%c`, `%x`, and `%X` are normalized to `%F %T`, `%F`, and `%T` so output stays locale-independent. Validate timezone offsets against ISO-8601's [-14:00, +14:00] range, preserve legacy long-fraction truncation in text readers, and use checked conversions for TIMESTAMP_TZ construction and explicit-offset adjustment. Parsing failures now surface the Chrono/range-specific error text used by the new backend. Add table-driven SQLLogicTests and scalar golden coverage for full-range values, formatted fields and errors, DATE interval result types, DST folds and gaps, midnight transitions, non-hour changes, fixed offsets, and range failures. Release benchmarks over 100,000 rows, measured against upstream/main with clean builds, CPU pinning, warmups, and alternating paired runs, show: - convert_timezone: 9.598 ms -> 6.659 ms (-30.5%) - timestamp_extract_components: 12.900 ms -> 9.133 ms (-29.2%) - timestamp_add_months: 14.720 ms -> 14.150 ms (-3.7%) - date_add_days: 67.16 us -> 67.16 us (no change) - string_parse_standard_to_date: 36.625 ms -> 14.245 ms (-61.2%) - string_parse_to_date: 26.290 ms -> 12.250 ms (-53.2%) - string_parse_to_timestamptz: 12.580 ms -> 3.126 ms (-75.2%) The explicit formatted-second range check adds about 10 ns/row in an A/B run (~0.6%), below observed run-to-run noise.
828002d to
3b71962
Compare
sundy-li
left a comment
There was a problem hiding this comment.
Reviewed the latest head. The %z offset validation issue is fixed and its thread is resolved. The AUTO conversion behavior was confirmed as pre-existing compatibility behavior, not a regression from this PR, and is tracked separately. All required CI checks pass.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Replace Jiff with Chrono and chrono-tz in SQL datetime parsing, formatting,
extraction, rounding, and arithmetic. Remove Jiff’s narrower absolute
timestamp ceiling and support the full SQL upper bound of
9999-12-31 23:59:59.999999UTC.Make
DATE +/- INTERVALandINTERVAL + DATEconsistently returnTIMESTAMP, independent of the interval unit or whether the interval isconstant or column-valued. Remove the planner rewrites that made the
result type depend on the interval literal.
Preserve month-end clamping and the established component ordering for
mixed intervals. Use calendar arithmetic for months and days and
elapsed-time arithmetic for microseconds. Share local-time resolution
across parsing and calendar arithmetic, preserve the source offset during
DST folds when valid, and handle gaps, midnight transitions, and non-hour
offset changes. Explicit-offset parsing and fixed-offset TIMESTAMP_TZ
arithmetic remain independent of session-local DST resolution.
Keep calendar inputs and SQL DATE values within years
0001..9999.Bound TIMESTAMP and TIMESTAMP_TZ by UTC instants within the same range.
Allow wider intermediate calendar values, but check completed INTERVAL
results against the SQL bounds and report overflow. Local timezone
rendering may reach year 0 or 10000 without extending the UTC value range.
Preserve existing clamp-to-MIN behavior in numeric conversions and legacy
arithmetic/AUTO parsing paths.
Adapt formatted datetime parsing and rendering to Chrono. Validate explicit
numeric offsets within
[-14:00, +14:00], including AUTO and fallbackparsing paths, and prevent invalid offsets from being silently accepted
by parser fallback.
Add and update scalar golden tests and SQL logic tests for constant and
column evaluation, result types, mixed intervals, timezone transitions,
explicit offsets, range boundaries, and folding/pruning behavior. Keep
boundary coverage active through string, numeric, and boolean assertions;
selected direct timestamp-result cases remain marked
TODO(bendsql)because of the driver’s Jiff-backed decoding limit.
Compatibility and scope
This changes DATE/INTERVAL result types and formatted datetime behavior,
including omitted-field defaults and format-directive support. Jiff
extensions
%Q,%:Q, and%Nare no longer supported, and%ZusesChrono’s numeric-offset rendering.
After 2099, named timezones use chrono-tz’s final recorded offset rather
than Jiff’s recurring-rule continuation.
Update numeric and timezone-sensitive domain inference where required by
these changes. The independently identified String-to-Date/Timestamp
pruning correctness issues remain out of scope for this PR.
Fix: #20416
Tests
Type of change
AI assistance
This change is