Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[C++] Split vector_selection.cc into more compilation units #35765

Closed
felipecrv opened this issue May 25, 2023 · 0 comments · Fixed by #35751
Closed

[C++] Split vector_selection.cc into more compilation units #35765

felipecrv opened this issue May 25, 2023 · 0 comments · Fixed by #35751
Assignees
@felipecrv
Labels
Component: C++ Type: enhancement
Milestone
13.0.0

Comments

@felipecrv
Copy link
Contributor

Describe the enhancement requested

vector_selection.cc contains multiple kernel implementations — take, filter, drop_null... — and many specializations. It's hard to see what is related to what and if we mix in kernel specializations for run-end encoded filters and data it will get really hard to review for completeness

Component(s)

C++
@felipecrv felipecrv self-assigned this May 25, 2023
@github-actions github-actions bot mentioned this issue May 25, 2023
Merged
1 task
@pitrou pitrou added this to the 13.0.0 milestone May 30, 2023
pitrou pushed a commit that referenced this issue May 30, 2023
@felipecrv
GH-35765: [C++] Split vector_selection.cc into more compilation units (…
44d1b61 
…#35751)

### Rationale for this change

When working on #35001 I had a hard time figuring where to place the code for all possible combinations of filters and REE data. `vector_selection.cc` is hard to follow with so many kernels implemented in a single file. This PR splits the two biggest ones: filter and take. Stuff that can be shared by both stays is in `vector_selection_internal.cc` and `vector_selection.cc` is concerned with the registering of the functions and a few smaller kernels.

### What changes are included in this PR?

- [x]  `vector_selection_(internal|take|filter).(cc|h)` source files were extracted from `vector_selection.cc`

### Are these changes tested?

Yes, by existing tests.

* Closes: #35765

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@felipecrv felipecrv

Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-35765: [C++] Split vector_selection.cc into more compilation units felipecrv/arrow
2 participants
@felipecrv @pitrou