fix(compute): detect temporal rounding overflow

[fallintoplace]

Rationale for this change

Temporal rounding can overflow while converting timestamps to nanoseconds, calculating the rounding interval, or multiplying a rounded quotient. The current unchecked arithmetic can return an incorrect timestamp.

What changes are included in this PR?

Use checked arithmetic for temporal conversions, interval setup, rounding results, calendar conversion, and origin adjustments. Propagate overflow as an Arrow invalid-value error.

Are these changes tested?

• go test ./arrow/compute

Are there any user-facing changes?

Inputs and options that cannot be represented during temporal rounding now return an error instead of a wrapped result.

[zeroshade]

Since this is a draft, i'll hold off on further review until it's marked ready and the conflicts are resolved

[zeroshade]

Found three blocking calendar-rounding issues: valid nanosecond results are rejected because helper boundaries are range-checked, wider temporal types are unnecessarily narrowed to nanoseconds, and long calendar periods still silently overflow through time.Duration midpoint arithmetic.

---

This review was drafted by an AI-assisted tool and confirmed by an Apache Arrow Go maintainer. After you've addressed the points above and pushed an update, an Apache Arrow Go maintainer — a real person — will take the next look at the PR. If you think one of the findings is misapplied, please reply on the PR and a maintainer will weigh in.

More on how Apache Arrow Go handles maintainer review:
CONTRIBUTING.md.

[zeroshade]

Blocking: Restricting this intermediate period boundary to the nanosecond timestamp window rejects inputs whose selected result is representable. For timestamp[ns] 2262-01-05 rounded half-up to one year, the correct result is 2262-01-01, but this checks the unused 2263-01-01 boundary and returns overflow. Likewise, 2201-01-01 rounded to 100 years should return 2200-01-01, but checking the 2300 boundary fails. Main succeeds in both cases. Please permit calendar boundaries outside the output window and validate only the selected result with timeToNanos; cover year, quarter, and month boundaries.

[zeroshade]

Blocking: Every calendar path is still narrowed to an int64 nanosecond intermediate, even when the input and correct result are representable in their original type. For example, flooring Date32(1500-06-15) to one year should return 1500-01-01, but this PR returns overflow. The same affects Date64 and second/millisecond/microsecond timestamps, whose ranges are much wider than timestamp nanoseconds. Please perform calendar arithmetic without narrowing to nanoseconds and range-check the final result in the input unit.

[zeroshade]

Blocking: halfRoundPeriod computes periodEnd.Sub(periodStart)/2; time.Time.Sub saturates at math.MaxInt64 for periods longer than about 292 years. Consequently, rounding 1948-01-01 to a 300-year multiple uses a midpoint in 1946 instead of the true 1950 midpoint and returns 2100 rather than 1800. This is pre-existing, but it is an unchecked calendar-rounding overflow directly within this PR’s scope. Please compute calendar midpoints without converting the entire period to time.Duration, with a regression test for a period over 292 years.

[zeroshade]

Thanks for the update. The earlier nanosecond-boundary and long-period midpoint examples are fixed, and the test suite passes locally. I found four remaining overflow/range cases that still need correction before this is safe across the supported temporal ranges.

[zeroshade]

Blocking: The fixed-duration slow path still rejects representable wider-unit values when the requested rounding is finer than the input resolution. For example, flooring a second-resolution 1500-06-15 12:34:56 timestamp to one nanosecond is an exact no-op, but this conversion returns invalid: temporal rounding overflow. Please avoid narrowing when the input is already aligned to the requested interval, and add coverage outside the timestamp-nanosecond window.

[zeroshade]

Blocking: time.Time.Unix() is undefined once a boundary lies outside the int64 seconds range, so this still rejects a representable selected result at the edge of timestamp[s]. With an input of January 2 in the year containing math.MaxInt64 seconds, half-up rounding to one year should select that year's representable January 1; the next-year boundary exceeds int64, these calls wrap, and checkedSubInt64 returns overflow. Please compute the midpoint without requiring both boundaries to fit Unix seconds and cover this edge.

[zeroshade]

Blocking: This reintroduces the same approximately 292-year saturation through time.Time.Sub. Flooring a second-resolution 1500-06-15 12:34:56 timestamp to one week returns 1677-09-19 instead of 1500-06-10 because the day distance saturates before division. Please calculate calendar-day/week distance without time.Duration, with wide-range week regression coverage.

[zeroshade]

Blocking: This does not actually validate second-resolution results: arrow.TimestampFromTime returns val.Unix() without a range check for arrow.Second. Ceiling math.MaxInt64 seconds to the next year therefore succeeds with wrapped value -9223372036852412416 instead of returning overflow. Please explicitly validate second-unit conversion (or make TimestampFromTime do so) and add a boundary test.

[zeroshade]

Thanks — all seven items from my previous rounds check out. I verified each against c78988c2 end-to-end rather than going off the test assertions, and they're genuinely fixed. I'll resolve those threads.

Two cases in the same class are still open, both in second-unit calendar rounding:

floor(Timestamp(math.MinInt64), unit=s, 1 year) → +9223372036825516800   err=nil
ceil (Timestamp(math.MaxInt64), unit=s, 1 day)  → -9223372036854745216   err=nil

A property sweep on the ordering invariant floor(x) ≤ x ≤ ceil(x) (4 units × 11 rounding units × 6 multiples) turns up 86 violations, all confined to unit=s at the int64 extremes, across year/quarter/month/day. The affected input range is exact, binary-searched rather than estimated: sec ∈ [MinInt64, -9223372036852412417]. The first value above that range correctly errors.

Root cause. The guard tests Year() > maxTimestampSecondYear, but the true maximum instant is 292277026596-12-04T15:30:07 — wrapped values that land later within that same year slip past it. The wrap is also bijective (tt.Unix() returns the original input), so the round-trip check can't see it either. That's exactly why the max-side year case (C7) is caught and the day case isn't.

Two dead ends worth passing on so you don't spend time on them:

• Don't compare instants. time.Unix(math.MaxInt64, 0) has an internally-overflowed ext field, so now.After(time.Unix(MaxInt64,0)) returns true and .Sub() returns garbage; its Year() only looks sane via a second wrap. Substituting an .After() check rejects ordinary timestamps and still leaves all 86 violations. The guard has to run in integer seconds at input conversion, before any time.Time exists.
• Don't put it in TimestampFromTime. That breaks TestTimestampFromTimeBoundaries/s, which asserts the wrapped value round-trips — the public API needs to stay bijective. The representability decision belongs in the kernel, which also avoids the blast radius on arrow/extensions/timestamp_with_offset.go:253, where the error is discarded into _.

Scope the input guard to the calendar path only — a blanket rejection breaks the sec/ms/us/ns no-ops that C4 asked for.

To be clear about severity: both of these are pre-existing on main, which returns 0 and -31536000 silently for the same inputs. This PR is a strict improvement, not a regression. I'm asking for them because they're the precise failure mode in the PR title, and the guard you already added covers the max-year half of it.

Please also add regression tests for Second+MinInt64 and Second+MaxInt64-with-day. The current matrix is asymmetric, which is why this slipped through: TestTemporalRoundingCalendarOriginAtMinimum uses MinInt64 but only in nanoseconds (year 1677, safe), and the second-unit tests only cover the max side. There is no Second+MinInt64 case — the one combination that's broken.

Two smaller things:

• The Lint job is failing only on formatting — 0 actual issues, the hook just rewrote files. gofmt -w arrow/compute/temporal_rounding_overflow_test.go clears it (the struct literal in TestTemporalRoundingNegativeCalendarMultiples is over-indented).
• Timezone handling is clean: I swept 7 zones including DST transitions, half-hour offsets and +14, and floor-to-day lands on local midnight in every one (0/161 non-midnight). Date32/Date64 extremes are clean too, and the Time32/Time64 wrap-to-midnight behaviour is byte-identical to the merge base, so it isn't this PR's concern.

On the diff size: it's justified. 55% is tests, there's no new exported API, the hunks are confined to the temporal rounding functions, and the restructuring was something I asked for in C2/C3/C6. Error-on-overflow via arrow.ErrInvalid matches Arrow C++, which returns Invalid with no null, saturating, or _checked variant.