Skip to content

feat: native RANGE window frames with explicit offset on " DECIMAL " ORDER BY#4987

Open
0lai0 wants to merge 2 commits into
apache:mainfrom
0lai0:feat-4834-range-offset-decimal-order-by
Open

feat: native RANGE window frames with explicit offset on " DECIMAL " ORDER BY#4987
0lai0 wants to merge 2 commits into
apache:mainfrom
0lai0:feat-4834-range-offset-decimal-order-by

Conversation

@0lai0

@0lai0 0lai0 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #4834 (DECIMAL ORDER BY only; DATE is handled in #4974)

Rationale for this change

Native windows used to fall back to Spark for RANGE frames with an explicit PRECEDING / FOLLOWING offset when ORDER BY was DECIMAL. The original reason was that Spark decimal arithmetic widens precision on +/-, so the computed boundary (e.g. Decimal(11,0)) no longer matched the current value's precision (e.g. Decimal(10,0)) and DataFusion's comparator failed with "Uncomparable values".

That mismatch was fixed upstream in apache/datafusion#22174 (closes #22113): ScalarValue::partial_cmp now ignores precision for Decimal128 when the scales match. The fix ships in DataFusion 54.0.0, which Comet already pins, so DECIMAL RANGE boundary arithmetic runs correctly on the native side with no planner changes needed. This PR just removes the now-stale Scala fallback. UNBOUNDED / CURRENT ROW already ran natively; DATE still falls back.

What changes are included in this PR?

  • Scala serde: drop the DecimalType RANGE fallback; keep the DATE fallback.
  • No native planner change needed (upstream DataFusion already handles the boundary comparison).
  • Docs: remove DECIMAL from the RANGE fallback list in operators.md / roadmap.md, and document a Decimal128 near-max-precision overflow difference under "Known differences".
  • Tests: 12 cases in CometWindowExecSuite (bounds combinations, duplicate values, nulls, DESC ordering, zero scale, high precision/scale, negative values crossing zero, fractional offset, DECIMAL(38,0) max precision).

How are these changes tested?

./mvnw test -Dtest=none -Dsuites="org.apache.comet.exec.CometWindowExecSuite"

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant