apache / apache/datafusion-comet

Native unix_timestamp returns incorrect results for negative fractional timestamps

Open
#5,896 0 comments 0 reactions 0 assignees View on GitHub
bug requires-triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.