sunchao opened a new pull request, #5364:
URL: https://github.com/apache/datafusion-comet/pull/5364
## Which issue does this PR close?
Part of #5149. This PR fixes the `to_time` and `try_to_time` portion of the
umbrella issue; timestamp and timestamp-NTZ casts remain separate follow-ups.
## Why are the changes needed?
Spark 4.1 runs `to_time` and `try_to_time` through Comet's native time
parser, but that parser currently uses Rust's Unicode-aware whitespace trimming
instead of Spark's exact parsing rules. Consequently, valid
ASCII-control-character padding can be rejected, while Unicode whitespace such
as `U+00A0` and `U+3000` can silently produce a time value where Spark rejects
the input.
Spark's behavior also depends on parsing order: `UTF8String.trimRight`
removes only ASCII spaces before detecting an optional `AM`/`PM` suffix; the
remaining time segment then trims bytes `0x00`–`0x20` plus `0x7F`. A control
character immediately before `PM` is therefore valid, while the same character
after `PM` is invalid.
## What changes were proposed in this PR?
- Reuse the existing zero-allocation Spark-compatible `trim_all` helper in
the native time parser.
- Match Spark's two-stage whitespace handling, AM/PM suffix detection, and
leading-`T` rules.
- Preserve throwing behavior for `to_time` and null-returning behavior for
`try_to_time`.
- Add exhaustive native regression coverage for all 34 trimmed ASCII/control
bytes, Unicode whitespace, suffix placement, `T` prefixes, and mixed/null
batches.
- Add Parquet-backed Spark SQL regressions that exercise native execution
and expected parse errors.
- Remove the fixed time functions from compatibility documentation while
preserving the remaining timestamp-cast divergence.
## How was this PR tested?
```bash
cargo fmt --all -- --check
cargo test -p datafusion-comet-spark-expr --lib
cargo clippy -p datafusion-comet-spark-expr --tests -- -D warnings
./mvnw --no-transfer-progress -Pspark-4.1 test \
-Dtest=none \
'-Dsuites=org.apache.comet.CometSqlFileTestSuite sql-file:
expressions/datetime/to_time.sql'
```
All **632 native Rust tests passed**. Clippy and formatting checks passed,
and the Spark **4.1.3** integration suite passed with materialized ASCII
control bytes, Unicode whitespace, suffix-sensitive inputs, native operator
checks, and expected error cases. An additional differential run against Spark
4.1.3 covered **136,584 inputs** without finding a newly introduced production
mismatch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
