apache / apache/datafusion-comet
to_time / try_to_time: native parser rejects 'T12' and '12:30:45.' which Spark accepts
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
## Describe the bug
Comet's native `to_time` / `try_to_time` parser (`native/spark-expr/src/datetime_funcs/to_time.rs`) diverges from Spark 4.1's `SparkDateTimeUtils.stringToTime` on three inputs that have nothing to do with whitespace trimming. In the first two cases Spark returns a value and Comet raises `The input string '...' cannot be parsed to a TIME value` (or returns `NULL` for `try_to_time`), so enabling Comet turns a succeeding query into a failing one.
These were found while reviewing #5364, which fixes a separate whitespace-trimming divergence in the same function. They are pre-existing and are not caused by that PR.
### 1. `T`-prefixed hour with no minute component
```sql
SELECT to_time('T12'); -- Spark: 12:00:00 Comet: error
SELECT to_time('T1'); -- Spark: 01:00:00 Comet: error
SELECT to_time('T12 AM'); -- Spark: 00:00:00 Comet: error
```
In `parseTimestampString`, the `T` branch (`j == 0 && b == 'T'`) sets `justTime = true` and advances `i += 3`, so the following digits are accumulated directly into `segments(3)` (the hour). The loop then ends and `isValidDigits(3, 2)` passes, giving `hr = 12, min = 0, sec = 0`.
Comet's `parse_time_components` skips an optional leading `T` but then unconditionally requires a `:` after the hour:
https://github.com/apache/datafusion-comet/blob/main/native/spark-expr/src/datetime_funcs/to_time.rs#L188-L190
### 2. Trailing `.` with no fractional digits
```sql
SELECT to_time('12:30:45.'); -- Spark: 12:30:45 Comet: error
```
Spark's `isValidDigits` short-circuits on `segment == 6`, so the fractional-second segment is allowed to have zero digits. Comet's `parse_fractional` returns `None` when `count == 0`.
### 3. Fractional digits 7 through 9 are dropped
```sql
SELECT to_time('12:30:45.1234567');
```
Spark keeps fractional digits 7-9 as a sub-microsecond remainder in `segments(9)` and folds them into `nanoOfSecond`, storing `45045123456700` nanos. Comet's `parse_fractional` truncates at 6 digits and stores `45045123456000`.
This one may not be user-observable: `to_time` produces `TIME(6)`, and the existing test at `spark/src/test/resources/sql-tests/expressions/datetime/to_time.sql` (`SELECT to_time('00:00:00.1234567')`) passes today because `TIME(6)` formatting truncates the difference away. It needs a check for whether a widening cast or `extract(second from ...)` can expose the stored nanos before deciding whether to fix it.
## Steps to reproduce
Requires Spark 4.1 with `spark.sql.timeType.enabled=true`.
```sql
SELECT to_time('T12'), to_time('12:30:45.');
```
## Expected behavior
Comet should return the same values Spark does.
## Additional context
Found with a differential harness that ports Spark 4.1's `stringToTime` and `parseTimestampString` to Rust and compares them against `string_to_time` over ~107k generated inputs (30 core time strings crossed with all 34 `trimAll` bytes and seven Unicode whitespace codepoints in leading, trailing, doubled, interior and pre-suffix positions). After #5364 lands, these three are the only remaining divergence classes in that corpus.
Contributor guide
Assessment
This issue has not been assessed yet.