apache / apache/datafusion-comet
fix: string-to-timestamp does not trim ISO control characters, and leading '+' returns null under ANSI
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
## 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
`-`, 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.
Contributor guide
Assessment
This issue has not been assessed yet.