Skip to content

Reenable float aggregation pushdown for duckdb except for min() #9094

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
myrrc merged 1 commit into from
Jul 31, 2026
Merged

Reenable float aggregation pushdown for duckdb except for min() #9094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions vortex-bench/sql/vortex/init.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
COPY (
SELECT
i AS id,
i % 1000 AS col,
(i * 2654435761) % 100000 AS col2
(i % 1000)::INTEGER AS col,
((i * 2654435761) % 100000)::INTEGER AS col2
FROM range(2500000000) t(i)
) TO 'test.parquet' (FORMAT parquet);
7 changes: 6 additions & 1 deletion vortex-duckdb/src/convert/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,12 @@ impl Display for PushedAggregate {

impl PushedAggregate {
pub fn build(self, dtype: DType) -> VortexResult<Box<dyn DynAccumulator>> {
let opts = NumericalAggregateOpts::default();
let opts = if dtype.is_float() {
// duckdb treats nan as a real value, vortex defaults skip nans
NumericalAggregateOpts::include_nans()
} else {
NumericalAggregateOpts::default()
};
Ok(match self {
Self::Min => Box::new(Accumulator::try_new(Min, opts, dtype)?),
Self::Max => Box::new(Accumulator::try_new(Max, opts, dtype)?),
Expand Down
22 changes: 15 additions & 7 deletions vortex-duckdb/src/table_function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use vortex::array::arrays::StructArray;
use vortex::array::arrays::scalar_fn::ScalarFnArrayExt;
use vortex::array::arrays::struct_::StructArrayExt;
use vortex::array::optimizer::ArrayOptimizer;
use vortex::dtype::PType;
use vortex::error::VortexExpect;
use vortex::error::VortexResult;
use vortex::expr::Expression;
Expand Down Expand Up @@ -672,14 +673,21 @@ pub fn pushdown_projection_aggregates(
return Ok(false);
};

// TODO(myrrc): DuckDB treats NaN as a normal value ordered greater than
// everything which is substandard. vortex aggregations just skip nan.
// don't push aggregations on floats until resolved
// See slt/duckdb/nan_aggregates.slt.
let projection_id_usize: usize = projection_id.as_();
if bind_data.column_fields[projection_id_usize]
.dtype
.is_float()
let dtype = &bind_data.column_fields[projection_id_usize].dtype;

// duckdb's min() returns nan only when every value is nan.
// vortex's min() either ignores or counts nans.
// See slt/duckdb/nan_aggregates.slt.
if aggregate == PushedAggregate::Min && dtype.is_float() {
Comment on lines +680 to +682

@robert3005 robert3005 Jul 31, 2026

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.

fwiw the default is to skip nans so I think we can push it down?

@myrrc myrrc Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we include nans, min([nan,1,2]) = NaN, and duckdb says 1, because -NaN is treated same as NaN.
If we skip nan, min([nan,nan]) = NULL, and we need NaN.

@robert3005 robert3005 Aug 1, 2026

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.

thanks for spelling this out. I missed some of the detail. DuckDB semantics are truly... special.

return Ok(false);
}

// duckdb's sum() on i64/u64 extends to i128/u128 but vortex
// accumulators work on i64/u64 max.
if aggregate == PushedAggregate::Sum
&& dtype.is_primitive()
&& matches!(dtype.as_ptype(), PType::I64 | PType::U64)
{
return Ok(false);
}
Expand Down
6 changes: 0 additions & 6 deletions vortex-sqllogictest/slt/aggregates_edge_cases.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,6 @@ SELECT count(x), sum(x), min(x), max(x), avg(x) FROM '${WORK_DIR}/f-posinf.vorte
----
3 Infinity 1 Infinity Infinity

onlyif duckdb
query TT
EXPLAIN SELECT sum(x) FROM '${WORK_DIR}/f-null.vortex';
----
<REGEX>:.*UNGROUPED_AGGREGATE.*

query IRRRR
SELECT count(x), sum(x), min(x), max(x), avg(x) FROM '${WORK_DIR}/f-null.vortex';
----
Expand Down
27 changes: 22 additions & 5 deletions vortex-sqllogictest/slt/duckdb/aggregate_pushdown.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ include ../setup.slt.no
statement ok
COPY (
WITH cte AS (SELECT generate_series AS i FROM generate_series(100000))
SELECT (CASE WHEN i > 0 THEN i ELSE NULL END) AS i FROM cte
SELECT (CASE WHEN i > 0 THEN i::INTEGER ELSE NULL END) AS i FROM cte
)
TO '${WORK_DIR}/agg-pushdown.vortex';

Expand Down Expand Up @@ -60,7 +60,24 @@ SELECT count(), count(), count() FROM '${WORK_DIR}/agg-pushdown.vortex';
----
100001 100001 100001

# aggregate over scalar i.e. SELECT mean(strlen(str))
# + cte
# + view + recursive view
# + cte referencing any_value()
statement ok
COPY (SELECT * FROM (VALUES (9223372036854775807::BIGINT), (9223372036854775807::BIGINT)) t(i))
TO '${WORK_DIR}/agg-pushdown-h.vortex';

query I
SELECT sum(i) FROM '${WORK_DIR}/agg-pushdown-h.vortex';
----
18446744073709551614

statement ok
COPY (SELECT * FROM (VALUES ((-9223372036854775807 - 1)::BIGINT), ((-9223372036854775807 - 1)::BIGINT)) t(i))
TO '${WORK_DIR}/agg-pushdown-hmin.vortex';

query I
SELECT sum(i) FROM '${WORK_DIR}/agg-pushdown-hmin.vortex';
----
-18446744073709551616

statement error
COPY (SELECT * FROM (VALUES (1::HUGEINT), (2::HUGEINT), (NULL)) t(i))
TO '${WORK_DIR}/agg-pushdown-hh.vortex';
156 changes: 148 additions & 8 deletions vortex-sqllogictest/slt/duckdb/nan_aggregates.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright the Vortex contributors

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

include ../setup.slt.no

Expand All @@ -17,22 +18,161 @@ COPY (

query TT
EXPLAIN
SELECT count(*), count(x), sum(x), min(x), max(x), avg(x)
SELECT count(*), count(x), sum(x), max(x), avg(x)
FROM '${WORK_DIR}/nan-agg.vortex';
----
<!REGEX>:.*UNGROUPED_AGGREGATE.*

query IIRRR
SELECT count(*), count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/nan-agg.vortex';
----
4 3 NaN NaN NaN

query TT
EXPLAIN
SELECT min(x) FROM '${WORK_DIR}/nan-agg.vortex';
----
<REGEX>:.*UNGROUPED_AGGREGATE.*

query II
SELECT count(*), count(x) FROM '${WORK_DIR}/nan-agg.vortex';
query R
SELECT min(x) FROM '${WORK_DIR}/nan-agg.vortex';
----
4 3
1

query RR
SELECT sum(x), avg(x) FROM '${WORK_DIR}/nan-agg.vortex';
query TT
EXPLAIN
SELECT min(x), max(x) FROM '${WORK_DIR}/nan-agg.vortex';
----
NaN NaN
<REGEX>:.*UNGROUPED_AGGREGATE.*

query RR
SELECT min(x), max(x) FROM '${WORK_DIR}/nan-agg.vortex';
----
1 NaN

statement ok
COPY (
SELECT * FROM (VALUES ('-nan'::DOUBLE), (1.0), (2.0)) AS t(x)
) TO '${WORK_DIR}/neg-nan.vortex';

query IRR
SELECT count(x), sum(x), max(x) FROM '${WORK_DIR}/neg-nan.vortex';
----
3 NaN NaN

statement ok
COPY (
SELECT * FROM (VALUES ('nan'::DOUBLE), ('nan'::DOUBLE)) AS t(x)
) TO '${WORK_DIR}/all-nan.vortex';

query IRRR
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/all-nan.vortex';
----
2 NaN NaN NaN

query R
SELECT min(x) FROM '${WORK_DIR}/all-nan.vortex';
----
NaN

statement ok
COPY (
SELECT * FROM (VALUES
('inf'::DOUBLE),
('-inf'::DOUBLE),
(0.0),
(CAST(NULL AS DOUBLE))
) AS t(x)
) TO '${WORK_DIR}/mixed-inf.vortex';

query IRRR
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/mixed-inf.vortex';
----
3 NaN Infinity NaN

query R
SELECT min(x) FROM '${WORK_DIR}/mixed-inf.vortex';
----
-Infinity

statement ok
COPY (
SELECT * FROM (VALUES ('inf'::DOUBLE), (1.0), (2.0)) AS t(x)
) TO '${WORK_DIR}/pos-inf.vortex';

query RRR
SELECT sum(x), max(x), avg(x) FROM '${WORK_DIR}/pos-inf.vortex';
----
Infinity Infinity Infinity

statement ok
COPY (
SELECT * FROM (VALUES ('-inf'::DOUBLE), (1.0), (2.0)) AS t(x)
) TO '${WORK_DIR}/neg-inf.vortex';

query RR
SELECT sum(x), avg(x) FROM '${WORK_DIR}/neg-inf.vortex';
----
-Infinity -Infinity

statement ok
COPY (
SELECT * FROM (VALUES (CAST(NULL AS DOUBLE)), (CAST(NULL AS DOUBLE))) AS t(x)
) TO '${WORK_DIR}/all-null.vortex';

query IRR
SELECT count(x), sum(x), max(x) FROM '${WORK_DIR}/all-null.vortex';
----
0 0 NULL

query R
SELECT min(x) FROM '${WORK_DIR}/all-null.vortex';
----
NULL

statement ok
COPY (SELECT i::DOUBLE AS x FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-plain.vortex';

query TT
EXPLAIN
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-plain.vortex';
----
<!REGEX>:.*UNGROUPED_AGGREGATE.*

query IRRR
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-plain.vortex';
----
2000 2001000 2000 1000.5

query R
SELECT min(x) FROM '${WORK_DIR}/big-plain.vortex';
----
1

statement ok
COPY (SELECT CASE WHEN i = 1000 THEN 'nan'::DOUBLE ELSE i::DOUBLE END AS x
FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-nan.vortex';

query IRRR
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-nan.vortex';
----
2000 NaN NaN NaN

query R
SELECT min(x) FROM '${WORK_DIR}/big-nan.vortex';
----
1

statement ok
COPY (SELECT CASE WHEN i = 1 THEN 'inf'::DOUBLE ELSE i::DOUBLE END AS x
FROM generate_series(1, 2000) t(i)) TO '${WORK_DIR}/big-inf.vortex';

query IRRR
SELECT count(x), sum(x), max(x), avg(x) FROM '${WORK_DIR}/big-inf.vortex';
----
2000 Infinity Infinity Infinity

query R
SELECT min(x) FROM '${WORK_DIR}/big-inf.vortex';
----
2
Loading