fix: Handle wrapped aggregates in aggregate pushdown#3804
Merged
Conversation
mithuncy
requested review from
mdashti,
rebasedming and
stuhood
as code owners
mithuncy
added
the
Do Not Cherry Pick
mithuncy
force-pushed
the
fix/3757-wrapped-aggregates
branch
from
2feb2c8 to
e07610d
Compare
Aggregates wrapped in functions like COALESCE(COUNT(*), 0) were not being recognized for pushdown to Tantivy. The code only checked if the top-level expression was an Aggref. Changes: - Add find_single_aggref_in_expr() to walk expression trees and find nested Aggref nodes using PostgreSQL's expression_tree_walker - Update replace_aggrefs_in_target_list() to use expression_tree_mutator so wrapped Aggrefs are properly replaced with placeholder FuncExprs - Only returns an Aggref if exactly one is found (multiple aggregates in a single expression are not supported)
mithuncy
force-pushed
the
fix/3757-wrapped-aggregates
branch
from
e07610d to
b7f6044
Compare
stuhood
requested changes
Dec 22, 2025
- Use Const nodes with per-row projection (pdbscan pattern) to evaluate wrapped expressions - Reuse tuple slot in state to fix per-row memory leak - Switch to per-tuple memory context for ExecBuildProjectionInfo to prevent allocation leak - Add debug_asserts for contiguous non-resjunk targetlist assumption
stuhood
approved these changes
Jan 5, 2026
Move generic projection code to shared customscan/projections.rs. Apply review feedback: Vec over HashMap, early-continue, .any(), iter_ptr().
vrn21
pushed a commit
to vrn21/paradedb
that referenced
this pull request
Jan 7, 2026
## Summary Fixes paradedb#3757 - Aggregates wrapped in functions like `jsonb_pretty(pdb.agg(...))` were not recognized for pushdown. - Add `find_single_aggref_in_expr()` using `expression_tree_walker` to find nested Aggref nodes - Update `replace_aggrefs_in_target_list()` to use `expression_tree_mutator` for wrapped Aggrefs - Follows same pattern as window function handling in `window_agg.rs` ## Test plan - Added non-window wrapped aggregate tests to `fn_wrapped_agg.sql` (Tests 5-10) - Verified issue test cases: `jsonb_pretty(pdb.agg(...))`, `pdb.agg(...)->'values'` - Passes `cargo fmt`, `cargo clippy`, `cargo check`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3757 - Aggregates wrapped in functions like
jsonb_pretty(pdb.agg(...))were not recognized for pushdown.find_single_aggref_in_expr()usingexpression_tree_walkerto find nested Aggref nodesreplace_aggrefs_in_target_list()to useexpression_tree_mutatorfor wrapped Aggrefswindow_agg.rsTest plan
fn_wrapped_agg.sql(Tests 5-10)jsonb_pretty(pdb.agg(...)),pdb.agg(...)->'values'cargo fmt,cargo clippy,cargo check