apache / apache/datafusion-comet
Native unix_timestamp returns incorrect results for negative fractional timestamps
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
### 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_
Contributor guide
Research direction
Start in native/spark-expr/src/datetime_funcs/unix_timestamp.rs at SparkUnixTimestamp::invoke_with_args and compare the timestamp conversion with Spark's behavior for negative fractional microseconds. Review the native unix_timestamp regression tests and the referenced fix in #5789; done means typed TIMESTAMP and TIMESTAMP_NTZ inputs, including nulls and positive values, match Spark without changing whole-second results.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- data
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100