peterxcli opened a new pull request, #5682:
URL: https://github.com/apache/datafusion-comet/pull/5682
## Which issue does this PR close?
Closes #5674.
## Rationale for this change
Comet's string→timestamp parser is a table of anchored regexes plus an
offset-suffix fallback, while Spark's `SparkDateTimeUtils.parseTimestampString`
is a segment scanner with per-segment digit rules (`isValidDigits`). They
disagreed in four ways on the default-on path, for both `TIMESTAMP` and
`TIMESTAMP_NTZ`:
| input | Spark | Comet before |
|---|---|---|
| `2020-1-1`, `2020-01-01 12:34:5` | value (1–2 digit segments) | NULL /
ANSI error |
| `2020-01-01 12:34:56.` | value (empty fraction) | NULL / ANSI error |
| `2020-10-01Z`, `2020-01-01+05:30`, `2020-10-01 UTC` | NULL (zone only
after seconds) | value |
| `0002020-01-01 00:00:00` | NULL (timestamp years ≤ 6 digits) | value |
## What changes are included in this PR?
- Relax the shape regexes to Spark's `isValidDigits` rules: year `\d{4,6}`
(only `stringToDate`, ported by `date_parser`, allows 7 — `CAST(... AS DATE)`
is unchanged), month/day/hour/minute/second `\d{1,2}`, fraction `\.\d*`.
- Add `ends_with_seconds_segment()` and only honour a stripped zone suffix
when the remainder ends in a seconds or fraction segment, in both
`timestamp_parser` and `timestamp_ntz_parser`. Any other suffix placement falls
through with the unstripped value and is reported as malformed (NULL, or
`CAST_INVALID_INPUT` under ANSI).
- Previously accepted inputs keep bit-identical values; the common path is
unchanged (a direct match still skips suffix extraction; the gate only runs on
the fallback path).
Note for #5130 (single-scan classifier): the regex edits are digit-count
changes to the same 14 shapes, and the gate is a predicate on the classified
shape of the stripped remainder, so it should rebase onto the classifier as a
one-line `matches!` check.
Observed but left out of scope: `timestamp_parser` (TZ only) still returns
NULL for `'2021-11-22 10:54:27 +08:00'` (space before a bare offset) because it
does not `trim_end()` the remainder the way the NTZ parser does; Spark accepts
it. Worth a follow-up issue.
## How are these changes tested?
- Rust: `timestamp_parser_spark_segment_rules_test`,
`timestamp_ntz_parser_spark_segment_rules_test` and
`test_cast_string_to_timestamp_spark_segment_rules_array` cover every case in
the issue table in legacy/try/ANSI modes, regression values for previously
accepted shapes (zone forms `Z`, `+05:30`, ` UTC`, ` America/Los_Angeles` after
a time, negative year), 6-digit valid vs 7-digit invalid years, and
`date_parser` still accepting a 7-digit year. `cargo test -p
datafusion-comet-spark-expr`: 662 passed; `cargo clippy --all-targets
--workspace -- -D warnings` clean.
- Scala: `cast StringType to TimestampType - Spark segment rules` and the
`TimestampNTZType` counterpart in `CometNativeCastSuite` compare against Spark
in legacy, try and ANSI modes, with each malformed string in its own query so
every ANSI error is checked. `CometNativeCastSuite` on Spark 4.1.3: 170 passed,
0 failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
