fix: align string to timestamp parsing with Spark's segment rules

[peterxcli]

Which issue does this PR close?

Closes #5674.

Rationale for this change

Comet's string-to-timestamp regexes disagreed with Spark's segment scanner for both TIMESTAMP and TIMESTAMP_NTZ. Besides rejecting valid inputs, the fallback could silently return the wrong timestamp: in UTC, 2020-10-1 and 2020-12-1 were interpreted with a trailing -01:00 offset, producing 01:00 instead of midnight. Unicode fractions such as 2020-01-01 12:34:56.1٢٢٢ and T1:2:3.1٢٢٢ could panic when truncated at a non-character byte boundary.

The ASCII digit classes also improve throughput. In andygrove's review benchmarks comparing base 81d637b9b with 0a1fc76a0, canonical, microseconds, and offset_suffix times decreased by 37%, 44%, and 25%, respectively, with comparable NTZ/non-UTC improvements. The canonical/microseconds base-versus-base noise floor was below 1%. These are reviewer-reported measurements: [0-9] avoids the more expensive Unicode Nd class compiled by \d with regex's default features.

What changes are included in this PR?

• Match Spark's ASCII segment rules: 4–6 year digits, 1–2 digits for other components, and an optional empty fraction. DATE retains its separate 7-digit-year grammar.
• Accept zone suffixes only after seconds/fractions in both timestamp parsers. Trim the stripped remainder consistently, accepting 2021-11-22 10:54:27 +08:00 while still rejecting date-only zones.
• Use checked fraction slices in both decoders so a future regex change cannot restore the UTF-8 boundary panic.
• Extend Rust and Parquet-backed Scala regressions with two-digit months/one-digit days, negative years, and the Unicode panic inputs. Widen the timestamp fuzz alphabet with -, ., +, and Z.
• The wider fuzz corpus exposed explicit positive years being rejected by Comet. Track this separately in String-to-timestamp cast rejects explicit positive years accepted by Spark #5716 and the compatibility guide; exclude only bare +[0-9]{4,6} years from this fuzz comparison until fixed.
• Add benchmark shapes for one-digit segments, empty fractions, and rejected date-only zones; skip the invalid-only shape under ANSI.

How are these changes tested?

• Rust spark-expr unit suite: 662 passed, including direct decoder tests that bypass the regex guards and exact timestamp/NTZ checks for whitespace before an offset.
• Spark 4.1.3 full CometNativeCastSuite: 169 passed, 1 failed, exposing String-to-timestamp cast rejects explicit positive years accepted by Spark #5716. After the narrow known-case exclusion, all 7 string-to-timestamp tests passed, including both segment-rule suites and the widened fuzz test.
• make core, benchmark compilation, all 77 benchmark smoke cases, package-wide Clippy with -D warnings, cargo formatting, Maven Spotless/scalastyle, and git diff --check passed. The benchmark smoke run checks execution, not performance; the throughput figures above are the reviewer's A/B results.

[sunchao]

Correctness

This change brings the timestamp grammar closer to Spark 3.5 and 4.0. It permits one-digit month, day and time segments and an empty fractional segment, restricts years to four through six digits, and accepts a timezone suffix only after seconds or fractional seconds. The separate DATE parser is unchanged. I compared the TIMESTAMP and TIMESTAMP_NTZ paths, the Legacy/ANSI/Try wrappers, null propagation, timezone handling and the native array dispatch against Spark's parser and cast implementation.

There is one new [P2] correctness issue in the broadened digit classes, detailed inline. A newly accepted non-ASCII digit can pass the regex and then be replaced by a default component when integer parsing fails. That turns malformed column input into a real timestamp, including in ANSI mode, instead of following Spark's rejection semantics. I separated this regression from the older two-digit Unicode behavior and did not report the older behavior as introduced here.

Validation and CI

The focused Rust probe compiled the exact head/base production parser blocks, helper graph, error definitions and Arrow array entry points. It covered 117 main inputs plus separate Unicode and empty/error sets across Legacy, ANSI and Try and both Spark-version flags. The 2,040 main/Unicode evidence assertions and 60 empty/error checks include assertions confirming the reported defect. They are not a claim that every input matched Spark. Column-array checks are important here because literal casts can be folded by Spark before native execution.

At the final CI/discussion refresh, there were 65 successful and nine skipped checks. I also verified the tested merge's exact parents for the inspected Rust and Spark jobs. Those logs report 1,114 Rust tests passed with four skipped, 1,296 Spark 3.5 tests passed with 11 cancelled and 12 ignored, and 1,303 Spark 4.0 tests passed with three cancelled and 12 ignored. No failures were reported in those inspected test summaries. The custom Unicode input was exercised through native Arrow entry points, not through a local Spark/JNI run. I did not run a local full Comet workspace suite or a distributed query.

Performance

The new suffix predicate does not add a heap allocation, but zoned inputs can now be classified once by that predicate and again by the main regex dispatch. I checked this with two alternating-order component benchmark runs using 8,192-row arrays, nulls, repeated kernel calls and nine samples. Ordinary date and ISO cases were roughly unchanged. Zulu-suffix cases measured about 1.16–1.21 times the previous time, and numeric offsets about 1.07–1.10 times. Named-zone results varied more.

These are shared-host parser measurements without CPU pinning, not a Spark-versus-Comet or end-to-end throughput result. They identify the extra classification cost but do not establish a query-level regression. The existing regex-dispatch design remains the main source of repeated work. I did not infer a performance improvement from the expanded correctness coverage.

Design

Keeping the timezone-suffix rule in one predicate makes the intended Spark rule easier to inspect across TIMESTAMP and TIMESTAMP_NTZ. Preserving the separate DATE path also limits the behavioral scope. The year bound, empty-fraction behavior and suffix placement are understandable local changes rather than a new parsing architecture.

The important design invariant is that the shape recognizer and numeric decoder must accept the same alphabet. The inline fix can restore that invariant locally while retaining the intended one-digit support. A broader parser rewrite is not needed to address the demonstrated regression.

Abstraction & complexity

The patch reuses the existing parser table, conversion helpers and cast dispatch rather than adding a new public interface, dependency or configuration. The shared suffix helper represents a concrete rule used by both timestamp paths, so its level of abstraction is appropriate.

The remaining complexity comes from having regex recognition and component decoding as separate stages. I traced both stages instead of treating a regex match as proof that decoding succeeds. The existing fallback defaults make that separation observable for malformed digits, which is why the new grammar needs the ASCII constraint described inline.

[sunchao]

Correctness

Follow-up on bcd3803717ec3e6d24d7a30637251d2669307591 against base 81d637b9bf40a5be6f4f0c65ad6f497b34746e69. The four reported month/day/minute/second examples are fixed. The added cases use Parquet-backed columns, with each malformed input evaluated separately under ANSI mode.

[P2] The existing Unicode-digit finding remains partially unresolved. In the newly accepted short-date forms, 2020-1-1T٢ still becomes 2020-01-01 00:00:00, and 2020-1-1T1:2:3.٢ becomes 2020-01-01 01:02:03 (UTC; ٢ is U+0662). Both produce values for TIMESTAMP and TIMESTAMP_NTZ in Legacy, TRY and ANSI modes. The authoritative base and Spark return NULL in Legacy/TRY and reject them in ANSI. This continues the existing finding, rather than adding a duplicate inline comment.

The hour class in RE_HOUR and the fraction class in RE_MICROSECOND still use Unicode-aware \d; failed numeric conversion then defaults to zero. The maintained Spark 3.5 and 4.0 scanners accept only ASCII digits in these segments. Please make the remaining numeric classes consistent with that rule and add these mixed short-date cases to the column regressions.

Validation

Exact production parser/Arrow cast blocks from the base, previous head and current head reproduced both failures. Spark 4.0.4 Cast(BoundReference) and generated projections agreed on rejection. The 180-input probe produced 9,720 native result records and 1,080 Spark evaluations through each path; 2,250 ASCII/null comparisons were unchanged by this update. This was component validation; the broader corpus also contains existing mismatches and is not an all-pass compatibility claim. No local full Comet JNI suite or Spark 3.4/3.5/4.1 runtime validation was performed. Canonical Spark 3.4/4.1 source refs were unavailable.

CI logs confirm the Rust segment tests and both timestamp segment suites in the Linux Spark 3.5/4.0 expression jobs passed. They executed merge 48846aee, combining this head with 719cba11, not the authoritative base above. The fresh head-check read at 21:37 UTC reports 64 successful and nine skipped checks, with no failing or unfinished check in that list. That does not change the merge/base qualification above.

Performance

The update narrows existing regex character classes without adding another per-row scan, allocation or parser pass. No new performance concern was identified in this delta. The correctness probe is not a benchmark; no throughput improvement is claimed.

Design

Restricting the numeric grammar is a small, appropriate fix, but it needs to cover whole accepted shapes: shortening an ASCII month/day can currently expose an unchanged Unicode hour or fraction. Applying the same digit rule throughout those patterns closes this gap without changing timezone handling or adding a separate parser.

Abstraction & complexity

No new production abstraction is introduced. The existing shared malformed-input fixtures remain suitable for the additional cases, and separate ANSI evaluations ensure a failure in the first input does not hide later regressions.

[sunchao]

Correctness

Follow-up on 0a1fc76a02ae14228a6c521d6dda1949f8afc312 against base 81d637b9bf40a5be6f4f0c65ad6f497b34746e69. The remaining cases in the existing P2 are fixed. Both 2020-1-1T٢ and 2020-1-1T1:2:3.٢ now follow the malformed-input path for TIMESTAMP and TIMESTAMP_NTZ. No remaining or new P1/P2 was found in this update.

All 14 timestamp patterns now use ASCII digits, matching the maintained Spark 3.5 and 4.0 byte scanners. I traced the year, hour, fraction and time-only classes through direct matching, timezone-suffix classification and both inner dispatch tables. Numeric Unicode cannot pass those gates and then become a default component. The date parser, numeric decoders, dispatch and dependencies are unchanged. For ASCII input, the regex languages are equivalent to the previous head, preserving the existing widths, empty fractions, timezone handling and microsecond calculation.

Validation

The expanded Rust parser fixtures and Parquet-backed Scala fixtures include the remaining examples, Unicode years, zoned fractions and time-only forms. Each malformed Scala input is evaluated separately for both timestamp types, so an earlier ANSI failure cannot hide later inputs. The helper explicitly checks native operators for Legacy and compares TRY results and ANSI exceptions.

CI logs confirm the three Rust segment tests and both timestamp segment suites passed on Spark 3.5 and Spark 4.1. Those jobs tested merge 7db8334c, combining this head with 719cba11. The relevant parser, dependency and timestamp-test sections match the head; this is not a claim that CI tested the authoritative base above.

The local Spark 4.0.4 component probe covered 457 inputs with 2,742 evaluations through each of the interpreted and generated paths. All 266 focused Unicode inputs returned NULL in Legacy/TRY and CAST_INVALID_INPUT in ANSI for both types. Source comparison additionally verified all 14 ASCII grammars. No new local native parser build or full Comet JNI suite was run. The earlier native corpus remains historical and includes known pre-existing mismatches; maintained Spark 3.4/4.1 source gaps remain recorded.

At 2026-09-05T02:39:14.744Z, current-head checks reported 53 successful, 11 running and 7 skipped, with no failures. Full CI was still running at that cutoff.

Performance

The update narrows the existing regex classes without adding a scan, allocation or parser pass. ASCII inputs retain the same classification decisions. No new performance concern was found in this delta, and no throughput improvement is claimed; the correctness checks were not benchmarks.

Design

Applying the same digit alphabet to every accepted timestamp shape closes the gap between recognition and numeric decoding. The shared predicates now enforce that rule consistently across direct and zoned input, while preserving the separate DATE grammar. This is an appropriate local fix.

Abstraction & complexity

No new production abstraction, dependency or configuration is introduced. Extending the shared malformed-input fixtures keeps the change small, and the existing per-input ANSI queries provide useful regression coverage without another testing framework.

[andygrove]

I dug into this one because the regex edits looked like they might cost something, and it turns out the opposite is true. Benchmarking cast_string_to_timestamp at 81d637b against 0a1fc76, the cast gets a lot faster: -37% on the canonical shape, -44% on microseconds, -25% on offset_suffix, and comparable numbers across the NTZ and non-UTC groups. I did a base-vs-base run first to get a noise floor and those two shapes sit under 1%, so it isn't measurement noise. The cause is the \d to [0-9] swap rather than the segment rules, since regex comes in with default features and \d compiles to the Unicode Nd class, which is much more expensive to match than one ASCII range. Could you add that to the description? As written this reads as a pure strictness change, and the next person to touch these patterns has no way to know the ASCII classes are load-bearing for throughput as well as correctness. It would also be worth adding cases to cast_string_to_timestamp.rs for the shapes you have made reachable, since none of the six existing ones use 1-digit segments, an empty fraction, or a zone after a date-only value.

The description also undersells the fix. On main, CAST('2020-10-1' AS TIMESTAMP) returns 2020-10-01 01:00:00 in a UTC session rather than NULL, because RE_DAY needed a 2-digit day, so extract_offset_suffix read the trailing -1 as a -01:00 offset and RE_MONTH then matched the remaining 2020-10. 2020-12-1 does the same thing. Those are silently wrong values rather than NULLs, which makes them the most valuable regression tests in the whole set, and neither one is in SPARK_SEGMENT_RULE_VALID or sparkSegmentRuleTimestamps. Every valid case you listed is a 1-digit-month shape, and those only ever returned NULL. Could you add the 2-digit-month with 1-digit-day shape to both corpora? While you are in there, neither Scala list has a negative-year case, valid or malformed, even though -?[0-9]{4,6} is one of the edited patterns and the Rust lists do cover it.

Related, and also worth putting in the description: on main CAST('2020-01-01 12:34:56.1٢٢٢' AS TIMESTAMP) panics with "end byte index 6 is not a char boundary", because Unicode \d let RE_MICROSECOND match a fraction of multi-byte digits and parse_to_timestamp_info then byte-slices a &str. T1:2:3.1٢٢٢ reaches the same bug through parse_str_to_time_only_timestamp. Your change makes both unreachable, which is a bigger deal than stricter parsing. Since those two slices are now safe only because no pattern can hand them non-ASCII, and that invariant lives a few hundred lines away at the regex definitions, could we make them char-boundary-safe on their own so that a future pattern edit cannot resurrect the panic?

On the '2021-11-22 10:54:27 +08:00' case you left out of scope, I think it belongs in this PR. Adding let stripped = stripped.trim_end(); before the ends_with_seconds_segment call in timestamp_parser returns the Spark value, leaves 2020-10-01Z at NULL, and passes all 662 spark-expr tests. It is the same trim_end() that timestamp_ntz_parser already applies. As it stands the two paths return different things for that input, a NULL and a value, which cuts against the point of giving them one shared zone rule. If you would rather keep it separate that is fine, but then it needs a filed issue and a bullet under "Known result-value divergences" in docs/source/user-guide/latest/compatibility/index.md, next to the #5149 whitespace entry, or it will not get picked up.

For what it is worth, I checked parseTimestampString and isValidDigits at v3.4.4, v3.5.9, v4.0.1, v4.1.1 and master, and all five agree on every rule you encoded, including the timestamp year bound of 6 against 7 for stringToDate, the empty fraction, and the zone id only being captured inside the seconds or fraction segment. No compatibility objection from me on the behavior itself.

Last thing, and I think it is the real story here. Your note about the fuzz alphabet is the important one: timestampPattern being "0123456789/:T" + whitespaceChars with no -, . or + means it cannot generate a plausible date, which is why this whole family of mismatches sat behind a passing fuzz test. Any appetite for widening it? If it surfaces more mismatches, those are issues to file rather than a reason to leave the alphabet alone.

[peterxcli]

Addressed the follow-up:

• Added the ASCII-class throughput explanation to the description, attributing the benchmark numbers to your measurements, and documented the invariant beside the regexes.
• Added 2020-10-1 / 2020-12-1 to the Rust and Scala corpora, negative-year Scala cases, and both Unicode fraction panic examples. Checked slices in both decoders now prevent the boundary panic independently of the regexes; direct decoder tests cover that.
• Included trim_end() and exact TZ/NTZ regression values for the spaced offset.
• Added benchmark shapes for single-digit segments, empty fractions, and rejected date-only zones.
• Widened the fuzz alphabet with -, ., +, and Z. This exposed a separate positive-year mismatch (+7528, etc.), now filed as String-to-timestamp cast rejects explicit positive years accepted by Spark #5716 and documented in the compatibility guide. Only bare positive years are excluded pending that fix; the wider alphabet stays.

Validation: 662 Rust tests passed; make core, benchmark compilation, all 77 benchmark smoke cases, Clippy (-D warnings), and formatting/style checks passed. The Spark 4.1.3 full cast suite had 169 passes and the one positive-year failure; after excluding that tracked case, all 7 string-to-timestamp tests passed, including both segment-rule suites and the widened fuzz test.

[sunchao]

Correctness

Follow-up at 7c449fd9 against 81d637b9, covering the changes since 0a1fc76a. One new P2 remains: trimming the extracted suffix prefix newly admits a leading-whitespace T-time-only input on Spark 4 without reapplying its existing rejection guard. The inline comment traces the exact input and Legacy/TRY/ANSI behavior.

The two fraction get(..) guards are safe, and all 14 ASCII digit patterns remain unchanged, so the previous Unicode-digit P2 stays fixed. The added month10/12-day1, negative-year and spaced-offset fixtures improve coverage. Null handling, fractional truncation, component bounds and overflow paths are unchanged. The known positive bare-year mismatch is explicitly tracked by #5716; the narrow fuzz exclusion and compatibility entry do not fix it. Broader whitespace-set differences, including Unicode separators before a zone, remain covered by #5149; they are not duplicated as another inline.

I compared the maintained Spark 3.5 and 4.0 sources: 3.5 trims bytes before scanning, while 4.0 retains the original byte index and accepts a leading T only at index zero. NTZ rejects time-only input entirely. Maintained Spark 3.4/4.1 source refs were unavailable, which remains a coverage limitation.

Validation

The local Spark 4.0.4 oracle exercised 251 non-foldable column inputs through both Cast evaluation and generated projections: 1,506 results per route, covering both timestamp types and all three modes. The primary counterexample returns NULL in Legacy/TRY and CAST_INVALID_INPUT in ANSI. Comet's changed behavior is established by the source path; I did not run a new local Comet native/JNI build. A separate Rust standard-library trim probe is only whitespace-component evidence.

Current-head Rust CI and Spark 3.5/4.0/4.1 expression jobs passed the segment-rule tests. CI tested merge 6f3dc8a5 (parents 7190df63 and this head), not the authoritative base pair. The parser, dispatch and benchmark blobs, plus relevant Scala timestamp fixtures/helpers, match the head; the merge also contains unrelated numeric tests, package dependency edges and benchmark registrations. At 2026-09-05 21:23:07 UTC, the refreshed head checks report 8 skipped, 65 successful, 1 failed. All checks had completed at that cutoff. The failed macOS scans job records a JVM SIGSEGV; its cause and relationship to this change are unproven.

Performance

The benchmark adds single-digit segments, an empty fraction and an invalid date-only zone. It evaluates columns in prebuilt 8,192-row batches, retains null coverage, and appropriately skips invalid batches under ANSI. The safe fraction access adds no allocation, and the new trim runs only after successful suffix extraction.

The peer benchmark report measured base 81d637b9 versus the previous head 0a1fc76a: roughly 37% lower canonical time, 44% lower microseconds time and 25% lower offset-suffix time, with similar NTZ/non-UTC results. These are historical peer measurements; I did not independently benchmark 7c449fd9, and benchmark compilation/smoke success is not a timing result. No additional material performance finding in this follow-up.

Design

The local defensive fraction guards and expanded fixtures fit the existing parser design. The suffix normalization correctly enables legal space-before-offset datetimes, but that path must also preserve the wrapper's version-specific admission checks. Reapplying the Spark 4 leading-whitespace restriction to the extracted T-time-only prefix, with a column regression fixture, is a narrow fix.

Abstraction & complexity

No new abstraction, dependency or serialization layer is introduced. The existing regex recognition and component decoders remain easy to follow, and the benchmark extends the existing batch generator. No separate complexity finding.

[sunchao]

Correctness

Follow-up at 89e5ff5c against 81d637b9, reviewing the delta from 7c449fd9. The leading-whitespace P2 is fixed. The wrapper now rejects a leading-whitespace T prefix before any suffix extraction, so a timezone cannot hide the restriction. The reported TAB-prefixed input and fractional variant produce NULL in Legacy/TRY and an error in ANSI on Spark 4. Spark 3.5 retains its trim-first behavior, and NTZ still rejects time-only input.

The maintained Spark 3.5 and 4.0 sources support this distinction: 3.5 starts scanning after trimming the bytes, while 4.0 recognizes a leading T only at original byte index zero. The new guard cannot admit anything rejected by the old guard; it broadens rejection only to leading-whitespace T forms that Spark 4 rejects. I verified that all other production parser bytes are unchanged, including the 14 ASCII patterns, fraction guards, offsets, null handling and overflow paths. No new or remaining P1/P2 was found in this update.

The added Parquet-backed tests evaluate the reported input, a fractional case and a valid control separately for both timestamp types. The helper checks the native operator in Legacy and compares TRY results and ANSI errors. Current CI logs confirm the new test passed on Spark 3.5, Spark 4.0 and Spark 4.1; the native whitespace regression also passed.

Those jobs tested merge d9b1ef12, combining this head with c348f775, rather than the authoritative base above. Its complete parser blob, timestamp dispatch arms and Scala timestamp fixtures/helpers match the head. Its dependency graph is newer, including Arrow 59.3 versus 58.4 and DataFusion 55 versus 54.1; the regex, chrono and chrono-tz package blocks match. This qualifies the CI evidence.

Locally, 232 Spark 4.0.4 inputs produced 1,392 matching results through each of Cast evaluation and generated projections. A separate standard-library guard probe rejected 105 inputs on the Spark-4 flag, all consistent with Spark's rejection, and rejected none on the Spark-3 flag. These are component checks; I did not run a new local Comet native/JNI build. Maintained Spark 3.4/4.1 source refs remain unavailable. The final current-head CI refresh at 2026-09-05T22:49:34Z reported 62 successful, three running and seven skipped checks, with no failed check at that cutoff.

Performance

This delta replaces up to four anchored regex checks with a prefix check on the existing leading-whitespace path. It adds no allocation or parser pass. The benchmark code is unchanged from the previous review. No material performance concern was found; no new timing or end-to-end speedup is claimed.

Design

Checking the raw-input restriction in the outer wrapper is appropriate: it covers direct and zoned time-only forms before either parsing route can bypass the rule. The existing version flag keeps Spark 3.5 behavior intact, and the paired valid control checks that the fix does not reject ordinary zoned time-only input.

Abstraction & complexity

The prefix predicate simplifies the existing condition without introducing an abstraction, dependency or configuration. Reusing the column-cast helper and extending the native fixture provides focused coverage for the actual bypass, including separate invalid ANSI evaluations.