apache / apache/datafusion-comet

to_time / try_to_time: native parser rejects 'T12' and '12:30:45.' which Spark accepts

Open
#5,366 2 comments 0 reactions 1 assignee Claimed by @peterxcli View on GitHub
area:expressions bug correctness priority:critical temporal expressions
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

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.