Uh oh!
There was an error while loading. Please reload this page.
ARROW-13764: [C++] Support CountOptions in grouped count distinct - #11011
ARROW-13764: [C++] Support CountOptions in grouped count distinct#11011lidavidm wants to merge 7 commits into
Conversation
lidavidm
commented
Aug 26, 2021
I'm still digging into this, but I noticed the unit tests are especially slow in debug mode. This is because we make a 2 GiB size allocation in GrouperFastImpl::GetUniques, which also makes me worry that these tests might use too much memory for CI. |
Also, filtering is not the right tradeoff to make: I think we don't care too much about the ONLY_NULL case and in the other cases, counting nulls is not that big a deal. So I'll update this to do the filtering in Finalize(). |
lidavidm
commented
Aug 26, 2021
Changed to filter the results after consumption. For whatever reason, not filtering before feeding batches into the grouper doesn't cause the runtime/memory blowup. |
pitrou
left a comment
There was a problem hiding this comment.
Thank you. A couple comments, but looks mostly good to me.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
values->null_bitmap() may be null here, I think.
There was a problem hiding this comment.
Perhaps add a test case where none of the values is null?
There was a problem hiding this comment.
Also add that the setting doesn't apply to the grouping keys?
pitrou
commented
Aug 30, 2021
Really? Why does this happen? |
lidavidm
commented
Aug 30, 2021
I was trying to figure out why, but didn't get very far in untangling the internals before I decided that filtering before grouping was pointless anyways. This doesn't happen the current implementation, I might poke at this further later though. |
lidavidm
commented
Aug 30, 2021
Thanks for the suggestions. I believe I've addressed everything. |
This works by filtering the values (trading off time for memory usage).