From 4aba56fb76755996b84fdd770913b3b575f2f6ea Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Thu, 30 Jul 2026 17:38:43 +0100 Subject: [PATCH] Reenable float aggregation pushdown for duckdb except for min() --- vortex-bench/sql/vortex/init.sql | 4 +- vortex-duckdb/src/convert/expr.rs | 7 +- vortex-duckdb/src/table_function.rs | 22 ++- .../slt/aggregates_edge_cases.slt | 6 - .../slt/duckdb/aggregate_pushdown.slt | 27 ++- .../slt/duckdb/nan_aggregates.slt | 156 +++++++++++++++++- 6 files changed, 193 insertions(+), 29 deletions(-) diff --git a/vortex-bench/sql/vortex/init.sql b/vortex-bench/sql/vortex/init.sql index 99fa2657ce7..aee587a2d2a 100644 --- a/vortex-bench/sql/vortex/init.sql +++ b/vortex-bench/sql/vortex/init.sql @@ -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); diff --git a/vortex-duckdb/src/convert/expr.rs b/vortex-duckdb/src/convert/expr.rs index aee065a2a20..9d57159c36f 100644 --- a/vortex-duckdb/src/convert/expr.rs +++ b/vortex-duckdb/src/convert/expr.rs @@ -524,7 +524,12 @@ impl Display for PushedAggregate { impl PushedAggregate { pub fn build(self, dtype: DType) -> VortexResult> { - 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)?), diff --git a/vortex-duckdb/src/table_function.rs b/vortex-duckdb/src/table_function.rs index 9457f00bc78..4910e010e3e 100644 --- a/vortex-duckdb/src/table_function.rs +++ b/vortex-duckdb/src/table_function.rs @@ -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; @@ -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() { + 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); } diff --git a/vortex-sqllogictest/slt/aggregates_edge_cases.slt b/vortex-sqllogictest/slt/aggregates_edge_cases.slt index 20c4db38dd3..de756759cfc 100644 --- a/vortex-sqllogictest/slt/aggregates_edge_cases.slt +++ b/vortex-sqllogictest/slt/aggregates_edge_cases.slt @@ -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'; ----- -:.*UNGROUPED_AGGREGATE.* - query IRRRR SELECT count(x), sum(x), min(x), max(x), avg(x) FROM '${WORK_DIR}/f-null.vortex'; ---- diff --git a/vortex-sqllogictest/slt/duckdb/aggregate_pushdown.slt b/vortex-sqllogictest/slt/duckdb/aggregate_pushdown.slt index 2bf91d7f007..c12145e969e 100644 --- a/vortex-sqllogictest/slt/duckdb/aggregate_pushdown.slt +++ b/vortex-sqllogictest/slt/duckdb/aggregate_pushdown.slt @@ -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'; @@ -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'; diff --git a/vortex-sqllogictest/slt/duckdb/nan_aggregates.slt b/vortex-sqllogictest/slt/duckdb/nan_aggregates.slt index 8e2328996dd..59931886824 100644 --- a/vortex-sqllogictest/slt/duckdb/nan_aggregates.slt +++ b/vortex-sqllogictest/slt/duckdb/nan_aggregates.slt @@ -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 @@ -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'; ---- +:.*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'; +---- :.*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 +:.*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'; +---- +:.*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