apache / apache/arrow-rs

Consolidate bitwise operation implementations

Open
#8,806 4 comments 0 reactions 0 assignees View on GitHub
arrow enhancement
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 16h
Merged PRs (30d)
168

Description

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
I spent quite a while reading the code for various bitwise kernels while working
with @rluvaton on
- https://github.com/apache/arrow-rs/pull/8619

I found there are many different, somewhat overlapping functions that are spread over the codebase

This makes it
1. Hard to find the appropriate API
2. Harder to optimize the correct operations as it is not clear always where
the relevant code is is located

I also think my experience in https://github.com/apache/arrow-rs/pull/8793 suggests there is significant room for optimization,
and that it is important to have two sets of functions: Modify in place and return new buffer

I think part of the current state is somewhat due to history and some of these functions predated the Buffer APIs

**Describe the solution you'd like**
I would like the implementation for the "core" APIs to be clear:
1. Create a new `Buffer` from a unary / binary bitwise operation
2. Apply a unary/binary bitwise operation to an existing Mutable buffer in place

After https://github.com/apache/arrow-rs/pull/8619 we have 2

**Describe alternatives you've considered**

Thus, I propose we do the following:
1. Add new `Buffer::bitwise_unary` and `Buffer::bitwise_binary` functions (that do the same thing as `bitwise_bin_op_helper` and `bitwise_unary_op_helper`) but are easier to find and use, and consistently named with `PrimitiveArray::unary` and `PrimitiveArray::binary` functions: https://github.com/apache/arrow-rs/pull/8854
2. Add `BooleanArray::binary` and `BooleanArray::unary` functions that use the new Buffer functions internally
2. Deprecate `bitwise_bin_op_helper`, and `bitwise_unary_op_helper` in favor of the new Buffer methods
3. Deprecate special methods such as `buffer_bin_or` methods in favor of using the new Buffer methods directly
4. Consolidate bitwise kernel benchmarks

Then we'll basically have two core APIs:
1. Apply bitwise operations in place (via apply_bitwise_unary / apply_bitwise_binary introduced in https://github.com/apache/arrow-rs/pull/8619)
2. Create a new buffer with the result of bitwise operations (via `Buffer::bitwise_unary` / `Buffer::bitwise_binary`)

Then all other APIs will then be thin wrappers around the core APIs and we can spend optimziation and testing efforts on these two core APIs.

**Additional context**

Contributor guide

Open the contributing guide

Research direction

Start by reading the existing bitwise_bin_op_helper and bitwise_unary_op_helper entry points, the Buffer and MutableBuffer APIs, and the BooleanArray and PrimitiveArray unary/binary APIs referenced in the issue. Compare the related work in pull requests 8619 and 8854. Done means the core APIs are consolidated, special helpers are deprecated or wrapped, and the bitwise kernel benchmarks are consolidated.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design, data-engineering
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.