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++] Simplify CleanListOffsets #36317

Closed
felipecrv opened this issue Jun 27, 2023 · 0 comments · Fixed by #36316
Closed

[C++] Simplify CleanListOffsets #36317

felipecrv opened this issue Jun 27, 2023 · 0 comments · Fixed by #36316
Assignees
@felipecrv
Labels
Component: C++ Type: enhancement
Milestone
13.0.0

Comments

@felipecrv
Copy link
Contributor

Describe the enhancement requested

It takes output parameters, but it could return a Result<BufferVector> instead.

Component(s)

C++
@felipecrv felipecrv self-assigned this Jun 27, 2023
@github-actions github-actions bot mentioned this issue Jun 27, 2023
Merged
@pitrou pitrou added this to the 13.0.0 milestone Jun 27, 2023
pitrou pushed a commit that referenced this issue Jun 27, 2023
@felipecrv
GH-36317: [C++] Return a BufferVector from CleanListOffsets (#36316)
1e00e7a 
### Rationale for this change

`CleanListOffsets` is taking output parameters when it could return multiple buffers in a vector exactly how they are supposed to be used to construct the list-like array.

Besides that, having the logic for setting the validity buffer split between it and the caller is unnecessarily confusing.

### What changes are included in this PR?

 - Addition of a new constructor for `MapArray` that can accept a pre-allocated `BufferVector`
 - Removal of output parameter in `CleanListOffsets` and change of return type

### Are these changes tested?

Yes, by existing tests. A pre-existing `MapArray` ctor delegates to this new ctor, so all tests that exercise the old ctor also exercise the new.
* Closes: #36317

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-36317: [C++] Return a BufferVector from CleanListOffsets felipecrv/arrow
2 participants
@felipecrv @pitrou