Skip to content

GH-34382: [C++] Support more types in run_end_encode and run_end_decode functions - #34761

Merged
westonpace merged 15 commits into
apache:mainfrom
felipecrv:run_end_encode_extra
Apr 4, 2023
Merged

GH-34382: [C++] Support more types in run_end_encode and run_end_decode functions#34761
westonpace merged 15 commits into
apache:mainfrom
felipecrv:run_end_encode_extra

Conversation

@felipecrv

@felipecrv felipecrv commented Mar 28, 2023 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 28, 2023

Copy link
Copy Markdown

@felipecrv
felipecrv marked this pull request as ready for review March 30, 2023 01:22
@felipecrv
felipecrv requested a review from westonpace as a code owner March 30, 2023 01:22
@felipecrv
felipecrv force-pushed the run_end_encode_extra branch from c4dd93b to 8fdd200 Compare March 30, 2023 15:33
@felipecrv

felipecrv commented Mar 30, 2023

Copy link
Copy Markdown
Contributor Author

@westonpace @wjones127 this is ready for review.

@dinimar you asked what was needed to add more types in the issue and after almost 600 lines... here it is :)

@felipecrv

felipecrv commented Mar 30, 2023

Copy link
Copy Markdown
Contributor Author

@zeroshade

@felipecrv
felipecrv force-pushed the run_end_encode_extra branch from 8fdd200 to b955730 Compare March 30, 2023 15:52
@dinimar

dinimar commented Mar 30, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@dinimar you asked what was needed to add more types in the issue and after almost 600 lines... here it is :)

@felipecrv Luckily, I didn't start working on this issue. Didn't think it will require so much knowledge about the project)

@felipecrv
felipecrv force-pushed the run_end_encode_extra branch 2 times, most recently from a1f1822 to 37aa666 Compare March 30, 2023 18:37
@felipecrv

felipecrv commented Mar 31, 2023

Copy link
Copy Markdown
Contributor Author

CI failure seems unrelated.

felipecrv added 13 commits March 31, 2023 10:37
@felipecrv
Turn ReadValueImpl and WriteValueImpl into a single template class
9a241ce
@felipecrv
Encapsulate more logic in ReadWriteImpl
11bf346
@felipecrv
Make values array allocation more general
a577a84
@felipecrv
Encapsulate more of the ReadWriteValueImpl contruction
5e0fedc
@felipecrv
Support temporal/duration/internal types in REE kernels
b60a27d
@felipecrv
Inline GenerateTypeAgnosticPrimitive to prepare for the expansion of …
d1ddae3 
…types supported
@felipecrv
Fix the type matchers and run_end_decode OutputType::Resolver
41945fa
@felipecrv
Encapsulate ValueRepr comparison in ReadWriteValueImpl
9bc0335
@felipecrv
Add support for decimals and fixed-size binary types
d5b6568
@felipecrv
Use TypeTraits<RunEndType>::bytes_required()
f42f707
@felipecrv
Be clear about these pointers not having array offset applied
20409d3
@felipecrv
Cleanup the ArrayData/Buffer allocation functions
c32bf6e
@felipecrv
Add support for variable length binary types
410479a
@felipecrv
felipecrv force-pushed the run_end_encode_extra branch from 37aa666 to 410479a Compare March 31, 2023 13:37
wjones127
wjones127 requested changes Mar 31, 2023
View reviewed changes

@wjones127 wjones127 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 like a great addition! Have one small suggestion on the tests.

Comment thread cpp/src/arrow/compute/kernels/vector_run_end_encode_test.cc Resolved
@felipecrv
felipecrv requested a review from wjones127 March 31, 2023 20:22
@felipecrv

felipecrv commented Apr 3, 2023

Copy link
Copy Markdown
Contributor Author

@wjones127 I rebased this locally right now and tests builds without any issue. Could you merge it please?

@westonpace
westonpace merged commit 6d109fb into apache:main Apr 4, 2023
@ursabot

ursabot commented Apr 4, 2023

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = f02d351 and contender = 6d109fb. 6d109fb is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.56% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Failed ⬇️2.86% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 6d109fbf ec2-t3-xlarge-us-east-2
[Failed] 6d109fbf test-mac-arm
[Finished] 6d109fbf ursa-i9-9960x
[Failed] 6d109fbf ursa-thinkcentre-m75q
[Finished] f02d3511 ec2-t3-xlarge-us-east-2
[Failed] f02d3511 test-mac-arm
[Finished] f02d3511 ursa-i9-9960x
[Failed] f02d3511 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@felipecrv
felipecrv deleted the run_end_encode_extra branch April 4, 2023 13:19
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
@felipecrv @ArgusLi
apacheGH-34382: [C++] Support more types in run_end_encode and run_en…
d99ddd7 
…d_decode functions (apache#34761)

* Closes: apache#34382

Authored-by: Felipe Oliveira Carvalho <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wjones127 wjones127 wjones127 approved these changes
@westonpace westonpace Awaiting requested review from westonpace

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Run-end encoding and decoding: Add support for more types

5 participants

@felipecrv @dinimar @ursabot @wjones127 @westonpace