andygrove opened a new issue, #5165:
URL: https://github.com/apache/datafusion-comet/issues/5165
## Describe the bug
Two Spark-compatibility divergences in the native string-to-timestamp parsers
(`timestamp_parser` and `timestamp_ntz_parser` in
`native/spark-expr/src/conversion_funcs/string.rs`). Both are pre-existing;
they were found
while porting Spark's `DateTimeUtilsSuite` string-to-timestamp cases into
the Rust tests in
#5130, where they are asserted as-is with a comment pointing here so they
cannot silently
widen.
### 1. Leading and trailing ISO control characters are not trimmed
Spark trims whitespace **and ISO control characters** before parsing a
timestamp
(`SparkDateTimeUtils.getTrimmedStart` / `getTrimmedEnd`, which use
`UTF8String.isWhitespaceOrISOControl`). Comet's `timestamp_parser` uses
Rust's `str::trim`,
which only strips Unicode `White_Space`. Control characters such as U+0003
are therefore left
in place and the parse fails.
```sql
-- Spark: 2015-03-18 12:03:17
-- Comet: NULL
SELECT CAST(CONCAT('2015-03-18 12:03:17', CHAR(3)) AS TIMESTAMP);
```
Affects any C0 control that is not Unicode whitespace (U+0000–U+0008,
U+000E–U+001B, U+007F).
All 30 of the permutations generated by Spark's
`permuteWithWhitespaceAndControl` helper for a
valid timestamp diverge.
Comet's `date_parser` in the same file already trims both via its
`is_whitespace_or_iso_control`
helper, so this is also internally inconsistent — `CAST(... AS DATE)`
accepts input that
`CAST(... AS TIMESTAMP)` rejects.
### 2. A leading `+` that is not a year sign returns null under ANSI instead
of raising
`timestamp_parser` returns `Ok(None)` for a leading `+` that is not followed
by
`<digits>-`, regardless of eval mode. Under ANSI, Spark raises
`CAST_INVALID_INPUT`.
```sql
-- with spark.sql.ansi.enabled=true
-- Spark: CAST_INVALID_INPUT
-- Comet: NULL
SELECT CAST('+12:12:12' AS TIMESTAMP);
SELECT CAST('+' AS TIMESTAMP);
```
The returned value is correct in non-ANSI modes; only the ANSI error is
missing.
## Steps to reproduce
Run the queries above, or see `spark_string_to_timestamp_test` and
`spark_string_to_timestamp_full_range_test` in
`native/spark-expr/src/conversion_funcs/string.rs`, which document both
divergences at the
assertions that cover them.
## Expected behavior
Match Spark:
1. Trim whitespace and ISO control characters in `timestamp_parser` and
`timestamp_ntz_parser`, reusing the same predicate `date_parser` already
uses.
2. Raise `CAST_INVALID_INPUT` under ANSI on the leading-`+` rejection path
(both parsers have
an identical copy of it).
Removing the two divergence carve-outs from the ported tests in #5130 is the
verification: they
were written to pass unmodified once this is fixed.
## Additional context
Everything else in Spark's `DateTimeUtilsSuite` string-to-timestamp coverage
matches exactly,
including the `Long.MinValue` / `Long.MaxValue` microsecond boundaries and
negative years.
--
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]
