Repository navigation
GH-47400: [C++] Support adding/subtracting intervals to/from timestamps - #53325
DeviousCardi wants to merge 1 commit into
Conversation
…mestamps
Add add, add_checked, subtract and subtract_checked kernels for
timestamp +/- {month_interval, day_time_interval, month_day_nano_interval},
with and without a timezone (add also accepts the interval first).
Months and days are calendar units applied to the local time of the
timestamp, while the sub-day part of the interval is a fixed duration,
following pandas.DateOffset. add applies months, days, then the sub-day
part; subtract applies the sub-day part first. Adding months clamps the
day of month. Ambiguous or nonexistent resulting local times return an
error, and the checked variants detect overflow.
Implicit temporal promotion is no longer applied in arithmetic dispatch
when an argument is an interval, so that e.g. date32 + interval is not
silently cast to a timestamp.
|
|
github-actions
Bot
added
Component: C++
Component: Python
Component: Documentation
labels
|
@DeviousCardi Thanks for the PR, pls use the AI disclosure checklist as given in the Arrow PR template. |
|
|
|
@Reranko05 Thanks for the reminder. The PR description now includes the template's "Was AI used for this PR?" checklist and a description of the AI assistance, just above the issue link. |
|
|
| const int64_t offset = localizer.OffsetAtSys(FloorDiv(t, kUnitsPerSecond)); | ||
| const int64_t local = Add(t, offset * kUnitsPerSecond, st); | ||
| int64_t day = FloorDiv(local, kUnitsPerDay); | ||
| const int64_t time_of_day = local - day * kUnitsPerDay; |
There was a problem hiding this comment.
The intermediate calendar values can overflow even when the final timestamp fits.
For timestamp[ns], adding one day to INT64_MIN causes signed overflow here. Subtracting one day from INT64_MIN + 86400000000000 should return INT64_MIN, but the multiplication below reports overflow. Nonzero timezone offsets cause similar failures near the bounds.
Please avoid these intermediate overflows and add kernel regression tests for both bounds, including a nonzero timezone offset.
| // 2025-11-02 01:30 EST - 1 day = 2025-11-01 01:30 EDT | ||
| {"America/New_York", "2025-11-02T06:30:00", 0, 1, 0, "2025-11-01T05:30:00"}, | ||
| // Subtraction undoes the mixed addition above: 24 hours, then 1 day | ||
| {"America/New_York", "2025-03-10T13:30:00", 0, 1, 24 * kHour, |
There was a problem hiding this comment.
Could you add a paired test showing the difference between subtracting a mixed interval and adding its negative?
Starting at 2025-03-10 09:30 EDT:
- Subtracting 1 day + 24 hours gives 2025-03-08 09:30 EST.
- Adding -1 day -24 hours gives 2025-03-08 08:30 EST.
Please also show that subtracting one calendar day and adding -1 calendar day give the same result. This would make the component order and its effect across DST clear.
3 participants
Rationale for this change
There is currently no kernel for
timestamp + interval, so e.g. "add one day to a zoned timestamp, keeping the wall clock time" requires a round trip through pandas (see #47400).What changes are included in this PR?
New kernels for
add,add_checked,subtractandsubtract_checked:timestamp(unit, tz) ± month_interval / day_time_interval / month_day_nano_interval -> timestamp(unit, tz), for all four units, with or without a timezoneadd/add_checkedalso accept the interval as the first argumentSemantics follow the discussion in the issue (Pandas'
DateOffset):2025-03-08 09:30 EST + 1 day = 2025-03-09 09:30 EDT.day_time_interval, nanoseconds of amonth_day_nano_interval) is a fixed duration, so2025-03-08 09:30 EST + 24h = 2025-03-09 10:30 EDT.addapplies months, then days, then the sub-day part.subtractapplies the sub-day part first, so subtracting "1 day 24 hours" undoes adding it (the mixed-interval examples in the issue). Months are applied before days in both directions, as inDateOffsetand PostgreSQL.Invaliderror is returned, with the same messages as the existingZonedLocalizer::ConvertLocalToSys. This is also what pandas does for a tz-awareSeries/DatetimeIndex+DateOffset(a scalarTimestampsilently resolves instead). The issue doesn't settle this point. If wanted, a follow-up could add an option to chooseearliest/latest/shift-forward likeAssumeTimezoneOptions.timestamp[s]) returnsInvalid._checkedvariants returnInvalidon int64 overflow; the unchecked variants wrap around like the other arithmetic kernels.Calendar arithmetic uses 64-bit year arithmetic, so the full range of
timestamp[s]is supported. The timezone lookups use the existingLocateZone/time_zone::get_infomachinery fromtemporal_internal.h.ArithmeticFunction::DispatchBestno longer applies implicit temporal promotion when one of the arguments is an interval. Otherwisedate32 + intervalwould be silently cast totimestamp[s] + interval. Dates are not supported yet; I'd suggest doing them in a follow-up once we agree on whatdate + interval with a sub-day partshould return.Are these changes tested?
Yes:
scalar_temporal_test.cc): month-end clamping, leap years, negative intervals, nulls, all units and all three interval types, both argument orders. DST transitions are covered in America/New_York, Europe/London and Australia/Lord_Howe (30 min DST), including results that land just after a gap or overlap and results that fall inside one (errors), plus a fixed-offset timezone. Error tests cover sub-unit and overflow cases. The expected DST values were checked against pandas 3.0.6.test_compute.py): parity withpandas.DateOffset+Timedeltaacross timezones and units, and the same DST gap/overlap errors as pandas.Are there any user-facing changes?
Yes, new kernels for
add/subtract(documented incompute.rst). Existing kernels are unchanged.Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
PR code and description written by:
Reviewed before submission by:
I used an AI coding assistant (Claude) for this PR. It drafted most of the kernel code, the C++ and Python tests, and the docs paragraph. I reviewed all of it, and the expected DST values in the tests were checked against pandas independently of the kernel. The 64-bit calendar helpers (
DaysFromCivil/CivilFromDays) follow Howard Hinnant's public-domain date algorithms, which are linked in a comment.Written by Codex.