Repository navigation
GH-46285: [C++] Add support for Decimal32/64 and HalfFloat to run_end_encode/run_end_decode - #46286
Merged
amoeba merged 8 commits intoJul 2, 2025
Merged
Conversation
amoeba
commented
May 1, 2025
amoeba
commented
May 2, 2025
raulcd
reviewed
May 2, 2025
Member
Author
|
This is ready for a final review, I've re-requested a couple of reviews but all are welcome to take a look. |
pitrou
approved these changes
May 12, 2025
pitrou
left a comment
pitrou
left a comment
Member
There was a problem hiding this comment.
LGTM, just one minor suggestion
Member
|
@amoeba Would you like to update this PR according to the review comment above? |
raulcd
reviewed
Jun 5, 2025
Co-authored-by: Raúl Cumplido <[email protected]>
We actually only have to do this numpy dtype thing for float16s, my mistake
amoeba
force-pushed
the
feature/GH-46285--ree-decimal3264float16
branch
from
June 7, 2025 01:26
1fcaa89 to
de0d89e
Compare
Member
Author
pitrou
reviewed
Jun 10, 2025
pitrou
left a comment
pitrou
left a comment
Member
There was a problem hiding this comment.
Just an additional question, LGTM otherwise.
Member
Author
|
I resolved your question @pitrou (it was a good one!). Does what I did there look good? I'll merge this if so. |
pitrou
approved these changes
Jul 2, 2025
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 626d639. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Contributor
|
Really nice one @amoeba! |
Mytherin
pushed a commit
to duckdb/duckdb
that referenced
this pull request
Jul 24, 2025
These tests were commented out because, when they were added, PyArrow didn't support run-end-encoding on Decimal32/64 arrays. PyArrow>=21 does support this: apache/arrow#46286 So we can uncomment the tests. This patch passes for me locally.
Mytherin
added a commit
to duckdb/duckdb
that referenced
this pull request
Jul 24, 2025
These tests were commented out because, when they were added, PyArrow didn't support run-end-encoding on Decimal32/64 arrays. PyArrow>=21 supports this as of apache/arrow#46286 so the tests can be uncommented. With this patch, this test suite passes for me locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Adds support to run_end_encode, run_end_decode for Decimal32, Decimal64, and HalfFloat.
What changes are included in this PR?
Are these changes tested?
Yes.
Are there any user-facing changes?
These are worth including as a line-item in release notes.