Skip to content

feat: fix windows decimal casting frame#22174

Open
comphead wants to merge 1 commit into
apache:mainfrom
comphead:win_decimal_frame
Open

feat: fix windows decimal casting frame#22174
comphead wants to merge 1 commit into
apache:mainfrom
comphead:win_decimal_frame

Conversation

@comphead
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

RANGE window frames with a DECIMAL ORDER BY column crash at runtime:

SELECT COUNT(*) OVER (
  PARTITION BY 1
  ORDER BY cast(1 as decimal(10, 0)) DESC
  RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING
) FROM (SELECT 1);
-- Internal error: Uncomparable values: Decimal128(Some(0),11,0), Decimal128(Some(1),10,0).

Frame-bound arithmetic (value ± delta) widens the decimal result precision by 1 (Decimal(10,0) ± Decimal(10,0) → Decimal(11,0)). search_in_slice then compares the widened target against the
original ORDER BY column, and ScalarValue::partial_cmp rejects decimals whose precision differs — even when the scale matches and the underlying integer representation is directly comparable.

That precision-equality gate is also inconsistent with SQL semantics: DEC(10,0) 1 and DEC(20,0) 1 represent the same number and should compare equal.

What changes are included in this PR?

datafusion/common/src/scalar/mod.rs

  • ScalarValue::partial_cmp for Decimal32 / Decimal64 / Decimal128 / Decimal256: compare underlying values whenever scales match, regardless of declared precision. Different scales still
    return None (rescaling would be required).

datafusion/sqllogictest/test_files/window.slt

  • Regression block covering the reporter query verbatim plus ASC/DESC × PRECEDING/FOLLOWING, symmetric N PRECEDING AND N FOLLOWING, and a non-zero-scale DECIMAL(10,2) case over multi-row
    partitions.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) common Related to common crate labels May 14, 2026
Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @comphead, LGTM.

Comment on lines +595 to 604
(Decimal32(v1, _, s1), Decimal32(v2, _, s2)) => {
if s1.eq(s2) {
// Same scale means the underlying integer values share
// a common interpretation regardless of declared
// precision (arithmetic such as `add_checked` widens
// precision by 1 but does not change the numeric
// meaning).
v1.partial_cmp(v2)
} else {
// Two decimal values can be compared if they have the same precision and scale.
None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be desirable to also compare decimals of different scales.

# column and failed with "Internal error: Uncomparable values".
############################################################################

# Original reporter query.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: reported?

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

Labels

common Related to common crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows errors out when RANGE frame is decimal

2 participants