-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use filtered_null_mask
in CountGroupsAccumulator
and PrimitiveGroupsAccumulator
#11825
Conversation
cee9356
to
ad1c75e
Compare
ad1c75e
to
b9128eb
Compare
I ran some benchmarks which suggested this could be slowing queries down. I don't understand how that could be possible but I need to review it more carefully |
Hi, I am curious about the slower cases, I merge and run some benchmarks in my local , found some faster and no cases slower...
|
That is a good question -- I will try again |
Here are some numebrs I got
|
Really strange... I will check it again too |
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days. |
Which issue does this PR close?
Draft:
convert_to_state
forAVG
accumulator #11734cargo bench
benchmarksRationale for this change
convert_to_state
forAVG
accumulator #11734 which I think can be used to reduce duplication (and possibly might be faster) inconvert_to_state
What changes are included in this PR?
Use the
filtered_null_mask
andset_nulls
functions introduced in #11734 inCountGroupsAccumulator
PrimitiveGroupsAccumulator
Are these changes tested?
functionally by CI
Perormance benchmarks show now change
Are there any user-facing changes?
No