-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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.
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
exec: filtering aggregations #39241
Comments
@jordanlewis whats your status on the case operator? I feel like the strategy for repeating the batch as done there to keep adding to a selection vector could be good here. |
Status is that it's kinda on hold til the FK stuff gets finished, and hoping to get some more reviews on it too. I saw your review (thanks!) but it's kinda complicated and I'd appreciate some more scrutiny :) |
I agree though that your proposed strategy could work. It actually might be as simple as planning a selection right before the aggregation step actually runs. |
yeah, thats what I was thinking. if we just select out the rows that don't pass the filter then we might be good. No worries about my review -- i don't have much authority over the codebase yet. |
#50721 adds the support for filtering hash aggregation, but filtering ordered aggregation seems to be a lot harder (I think it might require plumbing the filtering logic right into the "heart" of each aggregate function). One edge case is what happens when the whole group is filtered out? We still need to let aggregate function populate the output for such group, otherwise there will be a misalignment between It's been a while since I struggled with that (but I did spend like a day trying to get it working in all cases), and here is a snippet of what I had in the end. I can't easily describe why I came to realization that there is no way around plumbing the FILTERing logic into the aggregate function because I forgot and lost all the context, but at the time I was certain of it. In the code snippet I was trying to modify the |
50721: colexec: add support for DISTINCT and FILTER hash aggregation r=yuzefovich a=yuzefovich This commit adds the support of DISTINCT and FILTERing hash aggregation. The approach is as follows: - to handle FILTER we run a selection operator on the input state - to handle DISTINCT we encode aggregation columns, one tuple at a time, and update the selection vector to include tuples we haven't yet seen - then we run the aggregation on the remaining selected tuples - and then restore the state with the original length and selection vector. Such handling of FILTER clause sounds good to me, but the handling of DISTINCT is somewhat unfortunate: we perform encoding one tuple at a time. Other approaches have been prototyped but showed worse performance: - using the vectorized hash table - the benefit of such approach is that we don't reduce ourselves to one tuple at a time (because we would be hashing the full columns at once), but the big disadvantage is that the full tuples are stored in the hash table (instead of an encoded representation) - using a single global map for a particular aggregate function that is shared among all aggregation groups - the benefit of such approach is that we only have a handful of map, but it turned out that such global map grows a lot bigger and has worse performance. Addresses: #39241. Addresses: #39242. Release note (sql change): Vectorized execution engine now natively supports DISTINCT and FILTERing hash aggregation. Co-authored-by: Yahor Yuzefovich <[email protected]>
We have marked this issue as stale because it has been inactive for |
Add filtering aggregator support to the vectorized engine. This is an extra filter step that only includes rows in the aggregation if they pass a filter.
Jira issue: CRDB-5586
The text was updated successfully, but these errors were encountered: