fix: Accept explicit positive years in timestamp casts

[peterxcli]

Which issue does this PR close?

Closes #5716.

Rationale for this change

Spark accepts explicit positive years such as +7528, but Comet returns NULL. Malformed signs also bypass ANSI errors.

What changes are included in this PR?

Accept either year sign in the shared timestamp patterns and remove the duplicated + handling from the TIMESTAMP and TIMESTAMP_NTZ parsers. Restore positive-year fuzz coverage.

How are these changes tested?

Rust and Parquet-backed SQL regressions cover Legacy, TRY, and ANSI casts, signed dates, timezone suffixes, and malformed signs. All 33 native string-cast tests and 14 targeted Spark 4.1.3 tests pass, including the 10,000-input fuzz test.

[sunchao]

Correctness

Reviewed 9577f1634f6d against 8320ae481b4e. The prior parser rejected bare positive years such as +7528 and could return null for malformed leading-plus input even in ANSI mode. The shared timestamp grammar now accepts one optional sign with 4–6 ASCII year digits. Removing the two prefilters lets invalid input reach the normal error path. The existing integer parser already accepts +, while calendar validity and checked microsecond conversion still enforce the value bounds.

This agrees with the maintained Spark 3.5/4.0 parser and Cast paths I inspected. LTZ uses the session or supplied zone. NTZ validates and ignores an allowed zone. Zone-position restrictions, fraction truncation and DST resolution are unchanged. DATE retains its separate parser and year-width rules. No new verified P1/P2 finding.

Validation

• The new SQL fixture uses Parquet columns and expect_native, covering both timestamp types, nulls, all seven segment shapes, UTC/+05:30, Legacy/Try/ANSI and individual malformed-sign errors. The 10,000-input timestamp fuzz test also restores the previously excluded positive years.
• I compared exact extracted base/head parser functions with Spark 4.1.3 utilities over 143 strings × 4 zones × 3 modes × 3 parser variants. The 5,148 observations include width, separators, overflow edges, leap dates, fractions, offsets and DST gaps/overlaps. The local Rust executable uses matching Chrono/regex versions but cached Arrow timezone code 58.4.0 instead of 59.3.0. it is not a full JNI/array-kernel build. NTZ ANSI errors are derived from the utility result and inspected wrapper semantics. Maintained Spark 3.4/4.1 source was unavailable.
• These checks do not establish full whitespace parity. Newly accepted bare positive years inherit the Unicode-trimming mismatch already pinned by the existing array test and tracked in #5149. The probe also reproduced unchanged NUL-trimming, empty-ANSI and extreme-negative-year/New York differences. These are recorded separately from the signed-year fix.
• The completed Rust CI job explicitly logs plus_sign_year_test and related timestamp/NTZ/array/offset/DST tests passing. Its merge commit has the assigned base/head parents and the same tree as this head. At 10:34:34 UTC, CI had 19 successful checks, 10 running and 7 skipped. Spark SQL fixture and restored fuzz execution remain unverified.

Performance

The production diff changes the leading regex character class and removes duplicated sign checks. It adds no new parsing stage, allocation or timezone lookup, and keeps the existing lazy regex initialization. I did not benchmark throughput. there is no measured speedup claim.

Design

Handling the sign in the shared grammar keeps LTZ, NTZ and timezone-suffix validation consistent. It preserves year widths, delimiters and zone placement instead of maintaining separate acceptance rules in each entry point. The integer-parser comment explains why an additional plus-stripping step is unnecessary.

Abstraction & complexity

The change removes duplicated validation without adding a parser abstraction or configuration. Valid signed/unsigned pairs and a separate malformed-input table make the Rust tests easy to audit across modes. The column-based SQL coverage adds native-route assertions beyond direct parser tests, and restoring the fuzz inputs removes a targeted coverage exception.

[sunchao]

Rechecked 9577f163 → 3df7c9c5 against base f29a2361. The complete three-file signed-year contribution is byte-identical. The six-file increment exactly matches the incoming CI base update, which leaves timestamp parsing, cast routing, fixtures and the dependency lock unchanged. I rechecked the leading-plus/BCE/zero/year-range boundaries and LTZ/NTZ/ANSI paths against maintained Spark 3.5/4.0 source. No new or remaining verified P1/P2 findings. Preserving the existing approval attached to this head.

Current validation closes the earlier execution gap. The Rust job logs plus_sign_year_test passing. Expression-job logs for Spark 3.5, 4.0 and 4.1 show the signed-year SQL fixture passing in UTC and +05:30, plus the restored 10,000-input timestamp cast test. Their checkout is 3d0e7a49, with the assigned base/head parents and a tree identical to head. The Linux native producer and inspected consumer also agree on the artifact ID and digest.

At 2026-09-11 16:06:47 UTC, checks are 68 successful, 1 running and 8 skipped. Overall CI is still running. The local CI configuration checker passed. No local product test or benchmark was rerun. The earlier 5,148-observation utility probe remains historical component evidence, and the pre-existing Unicode-trimming differences tracked in #5149 remain unchanged. Maintained Spark 3.4/4.1 source is still unavailable. The current 4.1 job results are execution evidence for those tests, not a broader source-compatibility claim.

[andygrove]

Thanks for tracking this down. I checked the branch out and ran a wider corpus against Spark 4.1 locally: around 120 signed inputs, each paired with its unsigned counterpart, across TIMESTAMP, TIMESTAMP_NTZ and DATE, in UTC and America/New_York, plus the malformed signs under ANSI. Everything matched Spark exactly, so the grammar change looks right to me. I also checked that the bytes(j) == '-' || bytes(j) == '+' branch in parseTimestampString is identical in 3.4.3, 3.5.9, 4.0.4 and 4.1.3, so this is correct on every version we support, not just 4.x. CometNativeCastSuite passes here with the fuzz exclusion removed, and cargo bench --bench cast_string_to_timestamp shows no regression in any of its 77 cases.

Three things before it goes in.

The compatibility guide still describes this as a known divergence. docs/source/user-guide/latest/compatibility/index.md:142 has an "Explicit positive timestamp years" bullet linking #5716, and as far as I can tell that is the only remaining reference to the issue in the repo. Could you delete those three lines here, so the guide does not outlive the bug?

This also fixes the second half of #5165. Removing the prefilter is exactly what routes a leading + that is not a year sign to the normal ANSI error path, and both examples in that issue, CAST('+12:12:12' AS TIMESTAMP) and CAST('+' AS TIMESTAMP), now raise CAST_INVALID_INPUT on this branch. Since #5165 is still open and milestoned for 1.1.0, would you mention it in the description and narrow the issue down to just the ISO control character trimming? Otherwise it sits there looking half-broken and the next person re-derives this.

The third is about the malformed-sign fixture, which I left as an inline comment.

[andygrove]

The interesting thing the deleted prefilter was doing was keeping +-leading values away from extract_offset_suffix, which scans for the rightmost + or - and tries to read everything from there as a zone offset. Now that they reach it, the only thing stopping +05:30 from being read as an empty datetime in zone +05:30, or +1234+05:30 as year +1234 in zone +05:30, is the ends_with_seconds_segment guard. None of the entries in this list get that far: the closest, +12:12:12, is rejected earlier because parse_sign_offset sees a five-character minute field.

Could you add +05:30, +12:12, +1234+05:30, +-1234 and +7528 UTC? I ran all five locally and they agree with Spark today, in legacy, try and ANSI, so this is just pinning the branch the fix now leans on.

[andygrove]

Folding the sign into the shared patterns instead of pre-stripping it is the right simplification, and I traced the path to satisfy myself it is Spark-correct. In parseTimestampString on 4.1.3 the leading + or - is consumed unconditionally before any segment parsing, and the justTime branch is the only thing gated on yearSign being empty, which is why +12:12:12 stays null while +7528-01-01 does not. That also means this widens coverage a bit beyond what the description says: RE_YEAR and RE_MONTH now accept bare +2020 and +2020-01, which Spark accepts and the old strip_prefix logic rejected. Good. And parse_to_timestamp_info gets there via parse:: accepting the +, so the comment you added is doing real work for the next reader. LGTM.