andygrove commented on PR #5130:
URL:
https://github.com/apache/datafusion-comet/pull/5130#issuecomment-5143025202
> we probably can port all spark string2ts test cases here.
Good call — done in e14e086. Ported from `DateTimeUtilsSuite`: `test("string
to timestamp")`, `SPARK-35780: support full range of timestamp string`,
`SPARK-37326: stringToTimestampWithoutTimeZone with allowTimeZone`, and the
`stringToTimestamp` half of `SPARK-15379`, plus a port of the
`permuteWithWhitespaceAndControl` helper.
Two things I did differently from a literal transcription:
- Expected values are computed from a proleptic-Gregorian day count rather
than from the parser, so they don't depend on the code under test. That also
covers the years outside `chrono`'s range, which the `Long.MinValue`/`MaxValue`
microsecond boundary cases need — the test asserts those two land exactly on
`i64::MIN`/`i64::MAX`.
- Each case runs across all three eval modes and a few session timezones.
Spark's suite loops `ALL_TIMEZONES` but only exercises the `Option`-returning
function; adding the ANSI dimension is where the second finding below came from.
Every one of Spark's expectations holds except two divergences, both
identical on `main` and unrelated to the shape classifier. Filed as #5165:
1. **ISO control characters aren't trimmed.** Spark trims whitespace *and*
ISO control before parsing (`getTrimmedStart`/`getTrimmedEnd`);
`timestamp_parser` uses Rust's `str::trim`, which only strips Unicode
whitespace. So `CAST(CONCAT('2015-03-18 12:03:17', CHAR(3)) AS TIMESTAMP)` is
null in Comet and a timestamp in Spark — all 30 of the
`permuteWithWhitespaceAndControl` permutations for a valid input diverge.
Notably `date_parser` in the same file already trims both, so `CAST(... AS
DATE)` accepts input `CAST(... AS TIMESTAMP)` rejects.
2. **Leading `+` under ANSI.** `'+12:12:12'` and `'+'` return null in every
mode; Spark raises `CAST_INVALID_INPUT` under ANSI. The non-ANSI value is
correct.
I left both out of scope here to keep this PR behavior-preserving, and
asserted them at their current behavior with a pointer to #5165 so they can't
silently widen. The carve-outs in the ported tests are written so that removing
them is the verification for the fix.
`CometCastSuite` on the Spark 4.1 profile: 162 succeeded, 0 failed, 9
ignored. Rust suite is 545 tests (was 540).
--
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]
