Skip to content

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

Description

@andygrove

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

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

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

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.

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions