Skip to content

Commit b426232

Browse files
compheadcomphead
authored
feat: fix windows decimal casting frame (apache#22174)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#22113 . ## Rationale for this change `RANGE` window frames with a `DECIMAL` `ORDER BY` column crash at runtime: ```sql 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.
1 parent 6a57b22 commit b426232

2 files changed

Lines changed: 124 additions & 12 deletions

File tree

‎datafusion/common/src/scalar/mod.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -592,38 +592,39 @@ impl PartialOrd for ScalarValue {
592592
// any newly added enum variant will require editing this list
593593
// or else face a compile error
594594
match (self, other) {
595-
(Decimal32(v1, p1, s1), Decimal32(v2, p2, s2)) => {
596-
if p1.eq(p2) && s1.eq(s2) {
595+
(Decimal32(v1, _, s1), Decimal32(v2, _, s2)) => {
596+
if s1.eq(s2) {
597+
// Same scale means the underlying integer values share
598+
// a common interpretation regardless of declared
599+
// precision (arithmetic such as `add_checked` widens
600+
// precision by 1 but does not change the numeric
601+
// meaning).
597602
v1.partial_cmp(v2)
598603
} else {
599-
// Two decimal values can be compared if they have the same precision and scale.
600604
None
601605
}
602606
}
603607
(Decimal32(_, _, _), _) => None,
604-
(Decimal64(v1, p1, s1), Decimal64(v2, p2, s2)) => {
605-
if p1.eq(p2) && s1.eq(s2) {
608+
(Decimal64(v1, _, s1), Decimal64(v2, _, s2)) => {
609+
if s1.eq(s2) {
606610
v1.partial_cmp(v2)
607611
} else {
608-
// Two decimal values can be compared if they have the same precision and scale.
609612
None
610613
}
611614
}
612615
(Decimal64(_, _, _), _) => None,
613-
(Decimal128(v1, p1, s1), Decimal128(v2, p2, s2)) => {
614-
if p1.eq(p2) && s1.eq(s2) {
616+
(Decimal128(v1, _, s1), Decimal128(v2, _, s2)) => {
617+
if s1.eq(s2) {
615618
v1.partial_cmp(v2)
616619
} else {
617-
// Two decimal values can be compared if they have the same precision and scale.
618620
None
619621
}
620622
}
621623
(Decimal128(_, _, _), _) => None,
622-
(Decimal256(v1, p1, s1), Decimal256(v2, p2, s2)) => {
623-
if p1.eq(p2) && s1.eq(s2) {
624+
(Decimal256(v1, _, s1), Decimal256(v2, _, s2)) => {
625+
if s1.eq(s2) {
624626
v1.partial_cmp(v2)
625627
} else {
626-
// Two decimal values can be compared if they have the same precision and scale.
627628
None
628629
}
629630
}

‎datafusion/sqllogictest/test_files/window.slt‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6431,6 +6431,117 @@ FROM (
64316431
2 2
64326432
3 3
64336433

6434+
############################################################################
6435+
# RANGE frame with DECIMAL ORDER BY: decimal arithmetic widens the result
6436+
# precision (e.g. Decimal(10,0) ± Decimal(10,0) → Decimal(11,0)), which
6437+
# previously left the boundary target incomparable with the ORDER BY
6438+
# column and failed with "Internal error: Uncomparable values".
6439+
############################################################################
6440+
query I
6441+
SELECT COUNT(*) OVER (
6442+
PARTITION BY 1
6443+
ORDER BY cast(1 as decimal(10, 0)) DESC
6444+
RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING
6445+
) FROM (SELECT 1);
6446+
----
6447+
1
6448+
6449+
# ASC + PRECEDING
6450+
query RRR
6451+
SELECT a,
6452+
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW),
6453+
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW)
6454+
FROM (
6455+
SELECT CAST(1 AS DECIMAL(10,0)) AS a
6456+
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
6457+
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
6458+
)
6459+
ORDER BY a;
6460+
----
6461+
1 1 1
6462+
2 1 2
6463+
3 2 3
6464+
6465+
# ASC + FOLLOWING
6466+
query RRR
6467+
SELECT a,
6468+
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING),
6469+
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING)
6470+
FROM (
6471+
SELECT CAST(1 AS DECIMAL(10,0)) AS a
6472+
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
6473+
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
6474+
)
6475+
ORDER BY a;
6476+
----
6477+
1 1 2
6478+
2 2 3
6479+
3 3 3
6480+
6481+
# DESC + PRECEDING
6482+
query RRR
6483+
SELECT a,
6484+
first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW),
6485+
last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW)
6486+
FROM (
6487+
SELECT CAST(1 AS DECIMAL(10,0)) AS a
6488+
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
6489+
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
6490+
)
6491+
ORDER BY a DESC;
6492+
----
6493+
3 3 3
6494+
2 3 2
6495+
1 2 1
6496+
6497+
# DESC + FOLLOWING
6498+
query RRR
6499+
SELECT a,
6500+
first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING),
6501+
last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING)
6502+
FROM (
6503+
SELECT CAST(1 AS DECIMAL(10,0)) AS a
6504+
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
6505+
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
6506+
)
6507+
ORDER BY a DESC;
6508+
----
6509+
3 3 2
6510+
2 2 1
6511+
1 1 1
6512+
6513+
# Symmetric N PRECEDING AND N FOLLOWING
6514+
query RRR
6515+
SELECT a,
6516+
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING),
6517+
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING)
6518+
FROM (
6519+
SELECT CAST(1 AS DECIMAL(10,0)) AS a
6520+
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
6521+
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
6522+
)
6523+
ORDER BY a;
6524+
----
6525+
1 1 2
6526+
2 1 3
6527+
3 2 3
6528+
6529+
# Non-zero scale: Decimal(10,2)
6530+
query RRR
6531+
SELECT a,
6532+
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING),
6533+
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING)
6534+
FROM (
6535+
SELECT CAST(1.50 AS DECIMAL(10,2)) AS a
6536+
UNION ALL SELECT CAST(2.50 AS DECIMAL(10,2))
6537+
UNION ALL SELECT CAST(3.50 AS DECIMAL(10,2))
6538+
)
6539+
ORDER BY a;
6540+
----
6541+
1.5 1.5 2.5
6542+
2.5 1.5 3.5
6543+
3.5 2.5 3.5
6544+
64346545
############################################################################
64356546
# ROWS frame regression guard: huge offsets already saturate via
64366547
# saturating_sub / min(length), verify we keep that behavior.

0 commit comments

Comments
 (0)