Skip to content

feat: fix windows decimal casting frame #22174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
comphead merged 2 commits into from
May 14, 2026
+124 −12
Merged

feat: fix windows decimal casting frame #22174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c432e57
feat: fix windows decimal casting frame
comphead May 14, 2026
1e9ba45
feat: fix windows decimal casting frame
comphead May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions datafusion/common/src/scalar/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,38 +592,39 @@ impl PartialOrd for ScalarValue {
// any newly added enum variant will require editing this list
// or else face a compile error
match (self, other) {
(Decimal32(v1, p1, s1), Decimal32(v2, p2, s2)) => {
if p1.eq(p2) && s1.eq(s2) {
(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
Comment on lines +595 to 604
Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria May 14, 2026

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.

Copy link
Copy Markdown
Contributor Author

@comphead comphead May 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @nuno-faria would you mind to clarify. if I understood the comments correctly I jotted a decimal compare query with diff scales and it worked

> SELECT cast(1.50 as decimal(10, 2)) > cast(1 as decimal(10, 0));
+-------------------------+
| Float64(1.5) > Int64(1) |
+-------------------------+
| true                    |
+-------------------------+
1 row(s) fetched. 
Elapsed 0.017 seconds.

Copy link
Copy Markdown
Contributor

@nuno-faria nuno-faria May 14, 2026

Choose a reason for hiding this comment

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

Yeah I guess in practice different decimals will always be cast to a common type, so this won't be an issue.

}
}
(Decimal32(_, _, _), _) => None,
(Decimal64(v1, p1, s1), Decimal64(v2, p2, s2)) => {
if p1.eq(p2) && s1.eq(s2) {
(Decimal64(v1, _, s1), Decimal64(v2, _, s2)) => {
if s1.eq(s2) {
v1.partial_cmp(v2)
} else {
// Two decimal values can be compared if they have the same precision and scale.
None
}
}
(Decimal64(_, _, _), _) => None,
(Decimal128(v1, p1, s1), Decimal128(v2, p2, s2)) => {
if p1.eq(p2) && s1.eq(s2) {
(Decimal128(v1, _, s1), Decimal128(v2, _, s2)) => {
if s1.eq(s2) {
v1.partial_cmp(v2)
} else {
// Two decimal values can be compared if they have the same precision and scale.
None
}
}
(Decimal128(_, _, _), _) => None,
(Decimal256(v1, p1, s1), Decimal256(v2, p2, s2)) => {
if p1.eq(p2) && s1.eq(s2) {
(Decimal256(v1, _, s1), Decimal256(v2, _, s2)) => {
if s1.eq(s2) {
v1.partial_cmp(v2)
} else {
// Two decimal values can be compared if they have the same precision and scale.
None
}
}
Expand Down
111 changes: 111 additions & 0 deletions datafusion/sqllogictest/test_files/window.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6431,6 +6431,117 @@ FROM (
2 2
3 3

############################################################################
# RANGE frame with DECIMAL ORDER BY: decimal arithmetic widens the result
# precision (e.g. Decimal(10,0) ± Decimal(10,0) → Decimal(11,0)), which
# previously left the boundary target incomparable with the ORDER BY
# column and failed with "Internal error: Uncomparable values".
############################################################################
query I
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);
----
1

# ASC + PRECEDING
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW),
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW)
FROM (
SELECT CAST(1 AS DECIMAL(10,0)) AS a
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
)
ORDER BY a;
----
1 1 1
2 1 2
3 2 3

# ASC + FOLLOWING
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING),
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING)
FROM (
SELECT CAST(1 AS DECIMAL(10,0)) AS a
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
)
ORDER BY a;
----
1 1 2
2 2 3
3 3 3

# DESC + PRECEDING
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW),
last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 1 PRECEDING AND CURRENT ROW)
FROM (
SELECT CAST(1 AS DECIMAL(10,0)) AS a
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
)
ORDER BY a DESC;
----
3 3 3
2 3 2
1 2 1

# DESC + FOLLOWING
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING),
last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 1 FOLLOWING)
FROM (
SELECT CAST(1 AS DECIMAL(10,0)) AS a
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
)
ORDER BY a DESC;
----
3 3 2
2 2 1
1 1 1

# Symmetric N PRECEDING AND N FOLLOWING
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING),
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING)
FROM (
SELECT CAST(1 AS DECIMAL(10,0)) AS a
UNION ALL SELECT CAST(2 AS DECIMAL(10,0))
UNION ALL SELECT CAST(3 AS DECIMAL(10,0))
)
ORDER BY a;
----
1 1 2
2 1 3
3 2 3

# Non-zero scale: Decimal(10,2)
query RRR
SELECT a,
first_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING),
last_value(a) OVER (ORDER BY a ASC RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING)
FROM (
SELECT CAST(1.50 AS DECIMAL(10,2)) AS a
UNION ALL SELECT CAST(2.50 AS DECIMAL(10,2))
UNION ALL SELECT CAST(3.50 AS DECIMAL(10,2))
)
ORDER BY a;
----
1.5 1.5 2.5
2.5 1.5 3.5
3.5 2.5 3.5

############################################################################
# ROWS frame regression guard: huge offsets already saturate via
# saturating_sub / min(length), verify we keep that behavior.
Expand Down
Loading