Skip to content

repr, expr: roll leap-second timestamps into the next minute - #38109

Open
def- wants to merge 1 commit into
pr-per-53from
pr-per-63
Open

repr, expr: roll leap-second timestamps into the next minute#38109
def- wants to merge 1 commit into
pr-per-53from
pr-per-63

Conversation

@def-

@def- def- commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

chrono represents a parsed :60 second as second 59 with nanos >= 1e9. That value sorts before the following second, while the epoch family (extract epoch, casts to mz_timestamp, timestamp subtraction, date_bin, precision rounding) counts it at or past that second. The two orders disagree, which breaks the monotonicity contracts persist filter pushdown narrows ranges with. An interior leap value escapes the range derived from a part's endpoints, so a part holding one could be discarded even though it matches.

Closes: PER-63

Description

Roll :60 over into the next minute at the parse boundary, matching PostgreSQL, so the leap representation never enters a timestamp.

TIME keeps the leap representation. Rolling a time over would wrap it to 00:00:00 and reverse its ordering, and the whole-second leap time is provably harmless, because fractional leap seconds are rejected at parse for every type.

add_date_time was the other way in. It passed a leap-second TIME's raw nanos straight through, so DATE + TIME '23:59:60' still constructed the representation and disagreed with the equivalent timestamp literal. It now rolls over the same way.

Both rollovers can overflow chrono's range. TIMESTAMP '262142-12-31 23:59:60' parses, because that is chrono's maximum date and the leap value passes the HIGH_DATE check, and adding the rollover second then overflows. Overflow reports out of range in the timestamp, timestamptz, and DATE + TIME paths instead of panicking. The timestamptz path had been folding that overflow into an invalid-input-syntax detail string, so it now reports the same out-of-range error kind the timestamp path does.

Effect on the frozen storage source casts

The rollover sits in the shared inner parse functions, so it also reaches parse_timestamp_legacy and parse_timestamptz_legacy, the frozen wrappers behind the storage source casts CastStringToTimestamp and CastStringToTimestampTz. That is a deliberate exception to the stability contract in mz_storage_types::sources::casts, chosen rather than stumbled into, and it is recorded at those casts' eval arm.

Storage wants the rollover for the same reason SQL does. Source-cast output is exactly what filter pushdown reads back later, so scoping the rollover to the non-legacy path would have left the bug alive on the path that matters most, and would have made a TIMESTAMP '...:60' literal disagree with a source-ingested '...:60'.

No migration accompanies the change, because the inputs whose output differs cannot reach those casts. They are built only for a PostgreSQL source column of type timestamp or timestamptz (mz_sql::pure::postgres::pg_type_to_cast_func), so their input is PostgreSQL's own text output for that type. PostgreSQL normalizes :60 at input and has no leap representation to emit, which is confirmed against PostgreSQL 16: '2015-06-30 23:59:60'::timestamp stores and renders as 2015-07-01 00:00:00. A text column takes no cast at all, so user-supplied :60 text never routes here either.

CastStringToMzTimestamp shares the same parse but converts to epoch milliseconds, which already counted a leap value at the following second, so its output is unchanged.

Verification

leap_second_rollover_at_max_date_errors in strconv.rs covers the overflow at the maximum date and pins the error kind on both the timestamp and timestamptz paths.

timestamp_frozen_leap_second in casts.rs is the cross-release pin for the frozen storage casts. It asserts the rolled-over output for CastStringToTimestamp and CastStringToTimestampTz, the unchanged CastStringToMzTimestamp value, and the out-of-range error on the maximum date. It lives with the other error-snapshot tests, which are absolute rather than comparative, so it catches a future change to the frozen behavior in a way the parity tests cannot. The parity_timestamp and parity_timestamptz inputs also gained :60 cases, so the storage and SQL casts cannot silently diverge on them.

test/sqllogictest/explain/pushdown.slt pins the rolled-over values for timestamp and timestamptz literals, for the mz_timestamp cast that pushdown narrows on, and for DATE + TIME.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KUZqx24SBfhHeRdZPN76QR

chrono represents a parsed :60 second as 59 with nanos >= 1e9, a value
that sorts before the following second while the epoch family (extract
epoch, casts to mz_timestamp, timestamp subtraction, date_bin, precision
rounding) counts it at or past that second. That breaks the monotonicity
contracts persist filter pushdown narrows ranges with: an interior leap
value escapes the endpoint-derived range, so a part holding one could be
wrongly discarded. Roll :60 over at the parse boundary, matching Postgres,
so the leap representation never enters a timestamp. TIME keeps it:
rolling a time over would wrap to 00:00:00 and reverse its ordering, and
the whole-second leap time is provably harmless (fractional leap seconds
are rejected at parse for all types).

add_date_time is the other way a leap representation reached a timestamp.
It passed a leap-second TIME's raw nanos straight through, so DATE + TIME
'23:59:60' still constructed one and disagreed with the equivalent
timestamp literal. It now rolls over the same way.

Both rollovers can overflow chrono's range: TIMESTAMP '262142-12-31
23:59:60' parses, because that is chrono's max date and the leap value
passes the HIGH_DATE check, and adding the rollover second overflows.
Overflow reports out of range in the timestamp, timestamptz, and
DATE + TIME paths instead of panicking.

Closes: PER-63

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@def-
def- requested a review from a team as a code owner August 7, 2026 11:07
@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

PER-63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant