Skip to content

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

Open

Description

@andygrove

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.<name>.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

Activity

  1. 0lai0 commented on Jul 25, 2026

    @0lai0
    Contributor

    Thanks @andygrove. I think make it actually compatible would be better.
    I'm happy to take the upstream DataFusion work for this.

  2. 0lai0 commented on Jul 25, 2026

    @0lai0
    Contributor

    I looked into how Spark computes the RANGE frame bound, and I think it explains the divergence.

    Spark builds the bound expression based on the ORDER BY column's data type, in WindowEvaluatorFactoryBase.createBoundOrdering:

    1. DATE → DateAdd, which is a plain Int + Int (no addExact, no ANSI check), so it silently wraps on overflow.
    2. DECIMAL → DecimalAddNoOverflowCheck, which deliberately skips the overflow check, so it neither returns NULL nor throws.

    So Spark's bound compute is intentionally overflow-tolerant. The bound value is only used as a comparison key (not stored as a result), so wrapping / skipping the check is safe and keeps the frame boundary consistent with the actual row ordering.

    DataFusion computes the frame bound in window_state.rs using add_checked / sub_checked. On overflow these return Err, and the bound collapses to the partition edge, that's where the result diverges from Spark.

    So making the DATE and DECIMAL cases actually compatible would mean changing this overflow behavior on the DataFusion side.
    Note the collapse is a deliberate choice, so this would likely need to be an opt-in behavior rather than changing the default for all DataFusion users.

    Is this the right direction?
    If so, I'm happy to do the upstream DataFusion work.

  3. added this to the 1.0.0 milestone on Jul 27, 2026
  4. added
    priority:highCrashes, panics, segfaults, major functional breakage
    and removed
    priority:criticalData corruption, silent wrong results, security issues
    on Jul 29, 2026
  5. andygrove commented on Jul 29, 2026

    @andygrove
    MemberAuthor

    I change priority from criticial to high given that this is fairly unlikely.

  6. 1 remaining item

  7. 0lai0 commented on Jul 31, 2026

    @0lai0
    Contributor

    Thanks @andygrove
    only DATE can be fixed with just wrap-instead-of-Err, but DECIMAL needs the ANSI-mode work in apache/datafusion#23929.
    Happy to pick up the DATE part.
    Does that split sound right, or should we wait for the ANSI-mode discussion to settle first?

  8. modified the milestones: 1.0.0, 1.1.0 on Aug 17, 2026
  9. kazuyukitanimura commented on Sep 16, 2026

    @kazuyukitanimura
    Contributor

    @comphead let's follow up.

  10. modified the milestones: 1.1.0, 1.2.0 on Oct 12, 2026
  11. modified the milestones: 1.2.0, 1.3.0 on Oct 15, 2026
  12. 0lai0 commented on Oct 18, 2026

    @0lai0
    Contributor

    An update on where this stands, and a correction to the description.

    Correction. The description says Spark returns NULL under non-ANSI and throws under ANSI when a DECIMAL bound overflows. That is not what Spark does. WindowEvaluatorFactoryBase builds a DECIMAL bound with DecimalAddNoOverflowCheck, which neither returns NULL nor throws: it keeps the exact value and compares against it. So my earlier comment here, that DECIMAL needs the ANSI-mode work in apache/datafusion#23929, was based on the wrong premise.

    The actual divergence is the same for both types: when the native bound overflows, DataFusion takes the partition edge as the bound, which also takes in the NULL keys sorted at that edge, while Spark's exact bound leaves them out. For example, DECIMAL(38,0) values [0, N, NULL] with N = 9e37 and ORDER BY d ASC NULLS LAST RANGE BETWEEN N PRECEDING AND N FOLLOWING give [2,2,1] in Spark and [2,3,1] natively.

    DECIMAL. #4987 falls back when the largest value of the ORDER BY type plus the offset can exceed 2^127 - 1 unscaled, which only DECIMAL(38, s) with a very large offset reaches. Everything else runs natively.

    DATE. #4974 (merged) falls back for offsets that can wrap Spark's Int addition, but the native bound already overflows once it leaves chrono's date range (about year 262142), and that pulls in NULL keys the same way. # lowers the threshold to 92093340 days, the largest offset that keeps the bound inside chrono's range for every date in Spark's supported range.

    @andygrove I can't edit the issue body. Could you update it to the corrected Spark behavior? Once #4987 and # merge, the last one can close this issue.

  13. andygrove commented on Oct 19, 2026

    @andygrove
    MemberAuthor

    22. @andygrove I can't edit the issue body. Could you update it to the corrected Spark behavior? Once #4987 and # merge, the last one can close this issue.

    Thanks for the update @0lai0 . Looks like one issue/PR number is missing from and # merge?

  14. 0lai0 commented on Oct 20, 2026

    @0lai0
    Contributor

    Ohh sorry that it was a placeholder for the DATE follow-up PR, which isn't open yet. I've edited the comment, and I'll link the PR here once it's up. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:expressionsExpression evaluationbugSomething isn't workingcorrectnesspriority:highCrashes, panics, segfaults, major functional breakage

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions