-
Notifications
You must be signed in to change notification settings - Fork 1.8k
perf: improve ScalarValue::to_array_of_size for Boolean and some null values
#18180
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.
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
Conversation
|
Here is a related item: #18159 The CI test failure https://github.com/apache/datafusion/actions/runs/18657662446/job/53190612662?pr=18180 looks like it may be a flaky test (there is guarantee that shuffle does not leave the input array the same) I will restart it |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @rluvaton
|
I wonder if this will benefit from #16789 or if we could expand that sort of approach to more places. |
I'm not familiar with that PR, do you plan to replace all nulls creation? |
|
The basic idea is that the |
…ll values (apache#18180) ## Which issue does this PR close? N/A ## Rationale for this change make stuff faster ## What changes are included in this PR? 1. updated Boolean scalar to array to use existing functions for creating `BooleanArray` with the same value n times 2. Update None `Binary`/`BinaryView`/`FixedSizeBinary`/`LargeBinary` to use `new_null_array` instead of repeat ## Are these changes tested? Existing tests ## Are there any user-facing changes? Nope
…ll values (apache#18180) ## Which issue does this PR close? N/A ## Rationale for this change make stuff faster ## What changes are included in this PR? 1. updated Boolean scalar to array to use existing functions for creating `BooleanArray` with the same value n times 2. Update None `Binary`/`BinaryView`/`FixedSizeBinary`/`LargeBinary` to use `new_null_array` instead of repeat ## Are these changes tested? Existing tests ## Are there any user-facing changes? Nope
Which issue does this PR close?
N/A
Rationale for this change
make stuff faster
What changes are included in this PR?
BooleanArraywith the same value n timesBinary/BinaryView/FixedSizeBinary/LargeBinaryto usenew_null_arrayinstead of repeatAre these changes tested?
Existing tests
Are there any user-facing changes?
Nope