feat: native RANGE window frames with explicit offset on " DECIMAL " ORDER BY#4987
feat: native RANGE window frames with explicit offset on " DECIMAL " ORDER BY#49870lai0 wants to merge 4 commits into
Conversation
|
fixed ci test in window_function.sql |
|
Thanks @0lai0, this is a clean removal and I appreciate that you tracked down the exact upstream fix. (Reviewed with LLM assistance.) I confirmed the core claim: the comparator fix from apache/datafusion#22174 shipped in DataFusion 54.0.0, and we pin The overflow case needs to change from default-native to opt-in. Right now the DECIMAL RANGE path runs natively by default, and the near-max-precision overflow divergence is captured only in prose in Two ways to resolve it, in order of preference:
Documenting it as a known difference while still running native by default isn't something we can ship. Whichever route you pick, a test that actually pins the chosen behavior (rather than avoiding the edge) would be good to have. Two smaller points:
|
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
RANGEframes with an explicitPRECEDING/FOLLOWINGoffset whenORDER BYwasDECIMAL. 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_cmpnow ignores precision forDecimal128when the scales match. The fix ships in DataFusion 54.0.0, which Comet already pins, so DECIMALRANGEboundary arithmetic runs correctly on the native side with no planner changes needed. This PR just removes the now-stale Scala fallback.UNBOUNDED/CURRENT ROWalready ran natively; DATE still falls back.What changes are included in this PR?
DecimalTypeRANGEfallback; keep the DATE fallback.RANGEfallback list inoperators.md/roadmap.md, and document aDecimal128near-max-precision overflow difference under "Known differences".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?