Skip to content

ARROW-12759: [C++][Compute] Add ExecNode for group by - #10660

Closed
michalursa wants to merge 46 commits into
apache:masterfrom
michalursa:ARROW-12759-execnode-for-groupby
Closed

ARROW-12759: [C++][Compute] Add ExecNode for group by#10660
michalursa wants to merge 46 commits into
apache:masterfrom
michalursa:ARROW-12759-execnode-for-groupby

Conversation

@michalursa

Copy link
Copy Markdown
Contributor

Adding ExecNode for grouped aggregation.

@github-actions

Copy link
Copy Markdown

@bkietz
bkietz self-requested a review July 6, 2021 13:07

@bkietzbkietz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good so far. We need unit tests, of course

Comment threadcpp/src/arrow/compute/exec/exec_plan.cc Outdated
Comment threadcpp/src/arrow/compute/exec/exec_plan.cc Outdated
Comment threadcpp/src/arrow/compute/exec/exec_plan.cc Outdated
Comment threadcpp/src/arrow/compute/exec/exec_plan.h
@bkietz
bkietz marked this pull request as ready for review July 9, 2021 16:08
@bkietz
bkietzforce-pushed the ARROW-12759-execnode-for-groupby branch from 89a37fc to 8b31684CompareJuly 9, 2021 21:09
Comment threadcpp/src/arrow/compute/kernels/hash_aggregate.cc
using per thread copy of data structures and merging them at the end
bkietz added a commit that referenced this pull request Jul 13, 2021
This is a pretty trivial node but it's needed for completeness and will give bindings a pipeline breaker to experiment with until #10660 merges
Closes#10705 from bkietz/13313-Add-ScalarAggregateNode
Authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Signed-off-by: Benjamin Kietzman <bengilgit@gmail.com>
@bkietz
bkietzforce-pushed the ARROW-12759-execnode-for-groupby branch from 92fb25e to 3c17e2aCompareJuly 22, 2021 16:56
@bkietz

Copy link
Copy Markdown
Member

@pitrou care to take a look?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@michalursa@bkietz@lidavidm