unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
duckdb/duckdb
mid8/10
query-optimizationdata-structurestesting-strategy

Fix statistics propagation for window aggregate functions

Project: duckdb/duckdb | Language: C++ | Concepts: query-optimization, data-structures, testing-strategy | Difficulty: mid
Link: https://github.com/duckdb/duckdb/pull/24030

The problem

Some aggregate functions in DuckDB, like BITSTRING_AGG, require knowledge of input statistics (e.g., min/max values) to work correctly. While these statistics were properly propagated for regular aggregates, they were not for window aggregate functions. As a result, using a stats-dependent aggregate as a window function (e.g., BITSTRING_AGG(id) OVER (...)) failed with an error: Could not retrieve required statistics.

Why it's non-trivial

A naive fix might just try to pass statistics to window aggregates the same way as for regular aggregates. However, window aggregates are handled differently in the logical plan, and their expressions are not the same as regular aggregates. The fix needs to ensure that statistics are propagated and that the correct callbacks are invoked, even though window aggregates do not use BoundAggregateExpression directly. Additionally, some aggregate functions (like COUNT) can rewrite themselves during statistics propagation, so the code must update the logical plan accordingly.

The approach

1. Propagate statistics to window aggregate arguments and invoke the statistics callback

The main change is in src/optimizer/statistics/operator/propagate_window.cpp. The new code walks over each window function, and if it is an aggregate, it:

  • Propagates statistics to its argument children.
  • Calls the aggregate's statistics callback, synthesizing a BoundAggregateExpression as needed.
  • Handles the case where the aggregate function and its children may be rewritten by the callback.
+       // Aggregate window functions need their statistics callback invoked so that
+       // stats-dependent aggregates such as bitstring_agg receive their required statistics.
+       if (over_expr.AggregateFunction()) {
+           auto &agg_children = over_expr.GetChildrenMutable();
+           vector<BaseStatistics> child_stats;
+           child_stats.reserve(agg_children.size());
+           for (auto &child : agg_children) {
+               auto stat = PropagateExpression(child);
+               if (!stat) {
+                   child_stats.push_back(BaseStatistics::CreateUnknown(child->GetReturnType()));
+               } else {
+                   child_stats.push_back(stat->Copy());
+               }
+           }
+           auto &aggregate = *over_expr.AggregateFunctionMutable();
+           if (aggregate.GetCallbacks().HasStatisticsCallback()) {
+               AggregateStatisticsInput input(over_expr.BindInfo(), child_stats, node_stats.get());
+               const idx_t child_count = agg_children.size();
+               // The callback takes a BoundAggregateExpression; a window aggregate has none, so
+               // synthesise one over the same function and children. Callbacks may rewrite both
+               // (count replaces itself with count_star and drops its children), so write both back.
+               BoundAggregateExpression synthetic(aggregate, std::move(agg_children), nullptr, nullptr,
+                                                 over_expr.Distinct() ? AggregateType::DISTINCT
+                                                                      : AggregateType::NON_DISTINCT);
+               aggregate.GetCallbacks().GetStatisticsCallback()(context, synthetic, input);
+               agg_children = std::move(synthetic.GetChildrenMutable());
+               aggregate = synthetic.FunctionMutable();
+               removed_aggregate_children |= agg_children.size() < child_count;
+           }
+       }

What this does and why:

  • For each window aggregate, it collects statistics for each argument (agg_children).
  • If the aggregate function has a statistics callback, it creates a synthetic BoundAggregateExpression so the callback can operate as if it were a regular aggregate.
  • The callback may rewrite the aggregate function or its children (e.g., COUNT(col) to COUNT(*) if col is NOT NULL), so the code writes the changes back to the logical plan.

2. Add a regression test for window aggregate statistics propagation

A new test file is added: test/sql/window/test_window_agg_stats_propagation.test. This covers cases where:

  • BITSTRING_AGG is used as both a regular and a window aggregate.
  • COUNT(col) as a window aggregate is rewritten to COUNT(*) when the column is NOT NULL.
  • The rewrite does not happen when the column is nullable.
+query II
+SELECT id, BITSTRING_AGG(id) OVER (ORDER BY id ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)::VARCHAR AS w
+FROM bt
+ORDER BY id;
+----
+1	1

What this does and why:

  • Verifies that BITSTRING_AGG works as a window aggregate without explicit min/max statistics, confirming the fix.
+# count(column) over a NOT NULL column is rewritten to count_star with zero children during
+# statistics propagation; the rewrite must reach the window expression, not a discarded copy.
+query II
+SELECT id, COUNT(id) OVER () FROM cnt ORDER BY id;
+----
+1	3
+2	3
+3	3

What this does and why:

  • Checks that the statistics propagation rewrite for COUNT applies to window aggregates as well.

3. Register the new test in relevant test configs

The new test is added to stats_dependent_tests.json and verify_aggregate_state_export.json to ensure it's only run in configurations where statistics are available and to track aggregate state export coverage.

+        "test/sql/window/test_window_agg_stats_propagation.test"

From the review thread

hawkfish: Hi @marcodelpin - The code looks fine to me but there is a CI failure. Looks like the test fails when the optimiser is disabled?

marcodelpin: Thanks @hawkfish — sorry for the slow reply. That failure was addressed in 835abe1b2: the new test asserts that BITSTRING_AGG(col) works without explicit min/max precisely because the optimizer now propagates window-aggregate statistics — so under debug_disable_optimizer=true no statistics exist at all and the binder error is the expected property, not a defect. The test is now listed in test/configs/stats_dependent_tests.json ("Statistics are not set without optimizers", which disable_optimizer.json pulls in via inherit_skip_tests) and in verify_aggregate_state_export.json (pre-existing FIXME: bitstring_agg has no state-export callback), following the existing pattern of both lists.

Takeaways

  • When propagating metadata (like statistics) through a query plan, ensure all relevant logical operators—including window aggregates—are handled, not just the most common cases.
  • If a callback or transformation can rewrite part of the logical plan (e.g., aggregate expressions), always write the result back to the plan to avoid using stale or incorrect nodes.
  • Regression tests for subtle optimizer-dependent behavior should be registered in test configurations that reflect their dependencies, and should assert both the positive (works with optimizer) and negative (fails without statistics) cases.