Skip to content

Commit b226545

Browse files
myrrcmyrrc
committed
Reenable float aggregation pushdown for duckdb except for min()
1 parent b61deb6 commit b226545

3 files changed

Lines changed: 160 additions & 15 deletions

File tree

‎vortex-duckdb/src/convert/expr.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,12 @@ impl Display for PushedAggregate {
524524

525525
impl PushedAggregate {
526526
pub fn build(self, dtype: DType) -> VortexResult<Box<dyn DynAccumulator>> {
527-
let opts = NumericalAggregateOpts::default();
527+
let opts = if dtype.is_float() {
528+
// duckdb treats nan as a real value, vortex defaults skip nans
529+
NumericalAggregateOpts::include_nans()
530+
} else {
531+
NumericalAggregateOpts::default()
532+
};
528533
Ok(match self {
529534
Self::Min => Box::new(Accumulator::try_new(Min, opts, dtype)?),
530535
Self::Max => Box::new(Accumulator::try_new(Max, opts, dtype)?),

‎vortex-duckdb/src/table_function.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -672,14 +672,14 @@ pub fn pushdown_projection_aggregates(
672672
return Ok(false);
673673
};
674674

675-
// TODO(myrrc): DuckDB treats NaN as a normal value ordered greater than
676-
// everything which is substandard. vortex aggregations just skip nan.
677-
// don't push aggregations on floats until resolved
675+
// duckdb's min() returns nan only when every value is nan.
676+
// vortex's min() either ignores or counts nans.
678677
// See slt/duckdb/nan_aggregates.slt.
679678
let projection_id_usize: usize = projection_id.as_();
680-
if bind_data.column_fields[projection_id_usize]
681-
.dtype
682-
.is_float()
679+
if aggregate == PushedAggregate::Min
680+
&& bind_data.column_fields[projection_id_usize]
681+
.dtype
682+
.is_float()
683683
{
684684
return Ok(false);
685685
}

‎vortex-sqllogictest/slt/duckdb/nan_aggregates.slt‎

Lines changed: 148 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
# SPDX-License-Identifier: Apache-2.0
22
# SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
# Vortex aggregate pushdown must match DuckDB's NaN semantics
4+
# Vortex aggregate pushdown must match DuckDB's float semantics:
5+
# NULL is skipped, NaN is a real value
56

67
include ../setup.slt.no
78

@@ -17,22 +18,161 @@ COPY (
1718

1819
query TT
1920
EXPLAIN
20-
SELECT count(*), count(x), sum(x), min(x), max(x), avg(x)
21+
SELECT count(*), count(x), sum(x), max(x), avg(x)
2122
FROM '${WORK_DIR}/nan-agg.vortex';
2223
----
24+
<!REGEX>:.*UNGROUPED_AGGREGATE.*
25+
26+
query IIRRR
27+
SELECT count(*), count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/nan-agg.vortex';
28+
----
29+
4 3 NaN NaN NaN
30+
31+
query TT
32+
EXPLAIN
33+
SELECT min(x) FROM '${WORK_DIR}/nan-agg.vortex';
34+
----
2335
<REGEX>:.*UNGROUPED_AGGREGATE.*
2436

25-
query II
26-
SELECT count(*), count(x) FROM '${WORK_DIR}/nan-agg.vortex';
37+
query R
38+
SELECT min(x) FROM '${WORK_DIR}/nan-agg.vortex';
2739
----
28-
4 3
40+
1
2941

30-
query RR
31-
SELECT sum(x), avg(x) FROM '${WORK_DIR}/nan-agg.vortex';
42+
query TT
43+
EXPLAIN
44+
SELECT min(x), max(x) FROM '${WORK_DIR}/nan-agg.vortex';
3245
----
33-
NaN NaN
46+
<REGEX>:.*UNGROUPED_AGGREGATE.*
3447

3548
query RR
3649
SELECT min(x), max(x) FROM '${WORK_DIR}/nan-agg.vortex';
3750
----
3851
1 NaN
52+
53+
statement ok
54+
COPY (
55+
SELECT * FROM (VALUES ('-nan'::DOUBLE), (1.0), (2.0)) AS t(x)
56+
) TO '${WORK_DIR}/neg-nan.vortex';
57+
58+
query IRR
59+
SELECT count(x), sum(x), max(x) FROM '${WORK_DIR}/neg-nan.vortex';
60+
----
61+
3 NaN NaN
62+
63+
statement ok
64+
COPY (
65+
SELECT * FROM (VALUES ('nan'::DOUBLE), ('nan'::DOUBLE)) AS t(x)
66+
) TO '${WORK_DIR}/all-nan.vortex';
67+
68+
query IRRR
69+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/all-nan.vortex';
70+
----
71+
2 NaN NaN NaN
72+
73+
query R
74+
SELECT min(x) FROM '${WORK_DIR}/all-nan.vortex';
75+
----
76+
NaN
77+
78+
statement ok
79+
COPY (
80+
SELECT * FROM (VALUES
81+
('inf'::DOUBLE),
82+
('-inf'::DOUBLE),
83+
(0.0),
84+
(CAST(NULL AS DOUBLE))
85+
) AS t(x)
86+
) TO '${WORK_DIR}/mixed-inf.vortex';
87+
88+
query IRRR
89+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/mixed-inf.vortex';
90+
----
91+
3 NaN Infinity NaN
92+
93+
query R
94+
SELECT min(x) FROM '${WORK_DIR}/mixed-inf.vortex';
95+
----
96+
-Infinity
97+
98+
statement ok
99+
COPY (
100+
SELECT * FROM (VALUES ('inf'::DOUBLE), (1.0), (2.0)) AS t(x)
101+
) TO '${WORK_DIR}/pos-inf.vortex';
102+
103+
query RRR
104+
SELECT sum(x), max(x), avg(x) FROM '${WORK_DIR}/pos-inf.vortex';
105+
----
106+
Infinity Infinity Infinity
107+
108+
statement ok
109+
COPY (
110+
SELECT * FROM (VALUES ('-inf'::DOUBLE), (1.0), (2.0)) AS t(x)
111+
) TO '${WORK_DIR}/neg-inf.vortex';
112+
113+
query RR
114+
SELECT sum(x), avg(x) FROM '${WORK_DIR}/neg-inf.vortex';
115+
----
116+
-Infinity -Infinity
117+
118+
statement ok
119+
COPY (
120+
SELECT * FROM (VALUES (CAST(NULL AS DOUBLE)), (CAST(NULL AS DOUBLE))) AS t(x)
121+
) TO '${WORK_DIR}/all-null.vortex';
122+
123+
query IRR
124+
SELECT count(x), sum(x), max(x) FROM '${WORK_DIR}/all-null.vortex';
125+
----
126+
0 0 NULL
127+
128+
query R
129+
SELECT min(x) FROM '${WORK_DIR}/all-null.vortex';
130+
----
131+
NULL
132+
133+
statement ok
134+
COPY (SELECT i::DOUBLE AS x FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-plain.vortex';
135+
136+
query TT
137+
EXPLAIN
138+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-plain.vortex';
139+
----
140+
<!REGEX>:.*UNGROUPED_AGGREGATE.*
141+
142+
query IRRR
143+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-plain.vortex';
144+
----
145+
2000 2001000 2000 1000.5
146+
147+
query R
148+
SELECT min(x) FROM '${WORK_DIR}/big-plain.vortex';
149+
----
150+
1
151+
152+
statement ok
153+
COPY (SELECT CASE WHEN i = 1000 THEN 'nan'::DOUBLE ELSE i::DOUBLE END AS x
154+
FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-nan.vortex';
155+
156+
query IRRR
157+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-nan.vortex';
158+
----
159+
2000 NaN NaN NaN
160+
161+
query R
162+
SELECT min(x) FROM '${WORK_DIR}/big-nan.vortex';
163+
----
164+
1
165+
166+
statement ok
167+
COPY (SELECT CASE WHEN i = 1 THEN 'inf'::DOUBLE ELSE i::DOUBLE END AS x
168+
FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-inf.vortex';
169+
170+
query IRRR
171+
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-inf.vortex';
172+
----
173+
2000 Infinity Infinity Infinity
174+
175+
query R
176+
SELECT min(x) FROM '${WORK_DIR}/big-inf.vortex';
177+
----
178+
2

0 commit comments

Comments
 (0)