Satyr09 opened a new issue, #5896:
URL: https://github.com/apache/datafusion-comet/issues/5896
### Describe the bug
## Describe the bug
Comet's native `unix_timestamp` can return a different result from Spark
when a timestamp is before the Unix epoch and has a fractional second component.
The epoch is `1970-01-01 00:00:00 UTC`. Timestamps are stored in
microseconds, and `unix_timestamp` converts them to whole seconds. Spark
divides by 1,000,000 and drops the fractional part, rounding toward zero. The
affected Rust implementation uses floor division, which rounds down toward
negative infinity.
For example, with the session time zone set to UTC:
| Typed timestamp input | Seconds before conversion to an integer | Spark
result | Affected Comet result |
| --- | ---: | ---: | ---: |
| `1969-12-31 23:59:59.500000` | -0.5 | 0 | -1 |
| `1969-12-31 23:59:58.500000` | -1.5 | -1 | -2 |
Both queries succeed, but Comet's answer is one second too low. Positive
timestamps and exact whole seconds do not reveal this difference.
The affected conversion handles both `TIMESTAMP` and `TIMESTAMP_NTZ`,
including arrays with and without nulls. The input must already be a timestamp:
passing a string to `unix_timestamp` exercises a different execution path and
does not test this native conversion.
## Steps to reproduce
Use a Spark session configured with Comet and a native build containing the
floor-division implementation, such as the source at
`0249715725e767a65145b7757bf51f2863676552`.
Create a small Parquet table so the query reads a timestamp column instead
of allowing Spark to evaluate a constant expression in advance:
```sql
SET spark.sql.session.timeZone=UTC;
SET spark.comet.enabled=false;
CREATE TABLE unix_timestamp_rounding_repro (ts TIMESTAMP)
USING parquet;
INSERT INTO unix_timestamp_rounding_repro VALUES
(CAST('1969-12-31 23:59:58.500000' AS TIMESTAMP)),
(CAST('1969-12-31 23:59:59.500000' AS TIMESTAMP));
-- Spark reference result: -1, 0.
SELECT ts, unix_timestamp(ts) AS seconds
FROM unix_timestamp_rounding_repro
ORDER BY ts;
SET spark.comet.enabled=true;
-- With the affected native conversion: -2, -1.
SELECT ts, unix_timestamp(ts) AS seconds
FROM unix_timestamp_rounding_repro
ORDER BY ts;
```
For the second query, check that the projection containing `unix_timestamp`
runs in Comet. A query that falls back to Spark will not expose the bug. The
same conversion problem applies to a `TIMESTAMP_NTZ` column.
## Expected behavior
Native `unix_timestamp` should match Spark by truncating fractional seconds
toward zero. The second query should return `-1` and `0`, just like the first
query.
Null inputs should continue to return null, and results for positive
timestamps and exact whole seconds should remain unchanged.
## Additional context
The affected code is in
[`native/spark-expr/src/datetime_funcs/unix_timestamp.rs`](https://github.com/Satyr09/datafusion-comet/blob/0249715725e767a65145b7757bf51f2863676552/native/spark-expr/src/datetime_funcs/unix_timestamp.rs),
in `SparkUnixTimestamp::invoke_with_args`. The timestamp branches use:
```rust
div_floor(micros, MICROS_PER_SECOND)
```
Rust's ordinary integer division matches Spark's behavior here:
```rust
micros / MICROS_PER_SECOND
```
This is a pre-existing correctness bug found while reviewing
[#5789](https://github.com/apache/datafusion-comet/pull/5789). It also affects
the existing native timestamp path independently of the string codegen dispatch
change.
The fix and regression tests are already included in #5789 in commit
`eac00028c74124e9b3b18a6f7968ba4947a6d80c`. The tests cover negative and
positive fractional values, exact seconds, zero, nulls, and both timestamp
types. The Spark tests explicitly require native execution for the typed inputs.
[Validation of the
fix](https://github.com/Satyr09/datafusion-comet/actions/runs/34757375606)
passed all four native `unix_timestamp` tests, plus the temporal suite and
`unix_timestamp` SQL fixtures on Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and
experimental 4.2.0. This issue tracks the correctness bug separately from the
dispatch enhancement in #5577.
### Steps to reproduce
_No response_
### Expected behavior
_No response_
### Additional context
_No response_
--
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]
