apache / apache/datafusion-comet

fix: string-to-timestamp does not trim ISO control characters, and leading '+' returns null under ANSI

Open
#5,165 1 comment 0 reactions 1 assignee Claimed by @peterxcli View on GitHub
area:expressions bug correctness priority:medium
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.