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.

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

Add BooleanArray::from_unary and BooleanArray::from_binary #3258

Merged
merged 7 commits into from Dec 2, 2022

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Dec 2, 2022

Which issue does this PR close?

Part of #3259
Relates to #2594

Rationale for this change

What changes are included in this PR?

Promotes some implementation code from the comparison kernels to a first-class primitive

Are there any user-facing changes?

@tustvold tustvold requested a review from viirya December 2, 2022 16:59
@github-actions github-actions bot added the arrow Changes to the arrow crate label Dec 2, 2022
@tustvold
Copy link
Contributor Author

tustvold commented Dec 2, 2022

I've confirmed this makes no difference to the comparison benchmarks

tustvold added a commit to tustvold/arrow-rs that referenced this pull request Dec 2, 2022
/// Combines the null bitmaps of multiple arrays using a bitwise `and` operation.
///
/// This function is useful when implementing operations on higher level arrays.
pub fn combine_option_bitmap(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously this would return an error if called with an empty arrays.

In practice no codepaths could hit this, and so I removed it as it seemed unnecessary

/// Compares the null bitmaps of two arrays using a bitwise `or` operation.
///
/// This function is useful when implementing operations on higher level arrays.
pub(super) fn compare_option_bitmap(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function wasn't being used anywhere - so I opted to just remove it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Looks a bit weird to have this in tests although it looks like to be a utility function.

///
/// # Panics
///
/// This function panics if left and right are not the same length
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I specifically want this to be infallible, because I want a future try_from_binary to be able to return a user-provided error, instead of bundling it up in ArrowError.

@tustvold tustvold merged commit ecbb8c2 into apache:master Dec 2, 2022
@ursabot
Copy link

ursabot commented Dec 2, 2022

Benchmark runs are scheduled for baseline = 9abdb55 and contender = ecbb8c2. ecbb8c2 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants