apache / apache/datafusion-comet

RANGE window frame boundary overflow diverges from Spark for DATE and DECIMAL ORDER BY

Open
#5,022 5 comments 0 reactions 2 assignees Claimed by @comphead View on GitHub
area:expressions bug correctness priority:high
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

## Summary

Native `RANGE` window frames with an explicit `PRECEDING`/`FOLLOWING` offset diverge from Spark when the boundary arithmetic `current +/- offset` overflows. This affects both `DATE` ORDER BY (#4974) and `DECIMAL` ORDER BY (#4987).

## Behavior

On boundary overflow:

- **Spark** computes the boundary through normal expression evaluation and produces a concrete value.
- `DATE`: via `DateAdd`, which is plain `Int + Int` with silent wraparound (not `Math.addExact`, not ANSI-gated). It never throws and yields a wrapped date.
- `DECIMAL`: via normal decimal arithmetic, returning `NULL` under non-ANSI and throwing under ANSI.
- **Comet/DataFusion** computes the boundary via `ScalarValue::add`/`sub`. For both `Date32 + IntervalDayTime` and `Decimal128` near max precision, the arithmetic returns `Err` on overflow, and the window frame collapses the bound to the partition edge.

So the two engines produce different frames (and under ANSI, Spark throws where Comet does not).

## Practical likelihood

- `DECIMAL`: rare but plausible, needs values near `Decimal128` max precision.
- `DATE`: extremely remote, overflowing `Date32` needs an offset of roughly 2.1 billion days (~5.88 million years) from a realistic date.

## Options

1. Make the native boundary overflow match Spark (best): wrap for `DATE`, `NULL`/throw for `DECIMAL` depending on ANSI. This likely means upstream DataFusion work.
2. Mark the affected cases `Incompatible(Some(...))` and gate the native path behind `spark.comet.operator..allowIncompatible`, falling back to Spark by default. Follows the `CometDataWritingCommand` precedent.

Whichever route, a test at the type boundary should pin the chosen behavior rather than avoiding the edge.

## Related

- #4974 (DATE, currently runs native by default, undocumented)
- #4987 (DECIMAL, currently runs native by default, documented as a known difference)
- Both are part of #4834.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.