ARROW-11598: [Rust] Split buffer.rs in smaller files

[jorgecarleitao]

No semantic change, just spliting up large files for easier navigation.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-11598

[nevi-me]

Wouldn't this change be breaking to users who use the functions in ops, as they've been crate-public?

[alamb]

FWIW it might not be possible bit I think in general making Buffer and MutableBuffer etc less exposed is a good direction -- they become implementation details that can then be more freely changed without affecting users (e.g. it would make things like jorgecarleitao/arrow2#1 easier) .

In other words, making ops crate public might be a good thing (even if it is a potentially breaking one)

I realize the cat may already be out of the bag there, but I think being more deliberate about what interfaces are public and what are private would help developers and users going forward

[nevi-me]

making ops crate public might be a good thing

My view's that if someone's manipulating bits in a general way that's not related to Arrow, they shouldn't be using these ops. So I agree with not making them public.

After looking at https://doc.rust-lang.org/reference/visibility-and-privacy.html, I see that I used the wrong terminology.
As now making arrow::buffer::buffer_bin_or inaccessible to arrow's consumers, this is a semantic change.

@jorgecarleitao WDYT about returning it to pub use ops::*, and then we can address which of our laundry we hang to the public, in future?

[alamb]

Sorry @nevi-me -- I meant "making ops crate not public might be a good thing"

[alamb]

How about we just make another follow on PR that makes ops pub again?

[jorgecarleitao]

First, this PR is not merged yet; I just mistakenly pushed it to master and reverted it 😞 Anyways...

AFAI can tell, buffer_bin_or is currently not public; pub(super) just makes it public to the arrow crate in this context (as super of buffer.rs is arrow).

In master, I can't make the below compile:

extern crate arrow;

use arrow::buffer::Buffer;
use arrow::error::Result;
use arrow::buffer::buffer_bin_or;

fn main() -> Result<()> {
    let a = Buffer::from_slice_ref(&[1u32, 2u32]);

    let b = (&a | &a)?;
    Ok(())
}

However, And and Or are indeed public and this PR makes them private. I just pushed a change to revert that by exposing the implementations in mod.rs.

[codecov-io]

Codecov Report

Merging #9473 (37ac558) into master (356c300) will decrease coverage by 0.03%.
The diff coverage is 93.18%.

@@            Coverage Diff             @@
##           master    #9473      +/-   ##
==========================================
- Coverage   82.31%   82.28%   -0.04%     
==========================================
  Files         233      236       +3     
  Lines       54412    54417       +5     
==========================================
- Hits        44791    44778      -13     
- Misses       9621     9639      +18     

Impacted Files	Coverage Δ
rust/arrow/src/buffer/mutable.rs	90.68% <90.68%> (ø)
rust/arrow/src/buffer/ops.rs	91.66% <91.66%> (ø)
rust/arrow/src/buffer/immutable.rs	96.82% <96.82%> (ø)
rust/datafusion/src/error.rs	33.33% <0.00%> (-7.41%)	⬇️
rust/datafusion/src/scalar.rs	55.69% <0.00%> (-1.59%)	⬇️
rust/datafusion/src/execution/context.rs	90.09% <0.00%> (-0.18%)	⬇️
rust/arrow/src/compute/kernels/cast_utils.rs	92.50% <0.00%> (ø)
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 356c300...37ac558. Read the comment docs.

[alamb]

The idea looks good great to me! Thank you @jorgecarleitao . I think @nevi-me 's question about pub --> pub(crate) should probably be answered before merging this PR.

[alamb]

Stylistically, what do you think about calling this rust/arrow/src/buffer/buffer.rs -- I found rust/arrow/src/buffer/immutable.rs was a little confusing because there was nothing named Immutable in this module - I realize Buffer is immutable but the mismatch just stuck out to me.

[nevi-me]

This sadly creates module inception @alamb :( Clippy's not liking it

[alamb]

LOL

[nevi-me]

@jorgecarleitao this is merged, but I can't see it on master. And why does git say you've force-pushed apache:master? I wanted to start rebasing my work onto this

[jorgecarleitao]

@jorgecarleitao this is merged, but I can't see it on master. And why does git say you've force-pushed apache:master? I wanted to start rebasing my work onto this

Because I made a mistake and force-pushed this + an old master to master. I have since reverted all the mess I created. This is not merged; github just does not understand what just happened (which, to be fair, I did not make it easy).

[nevi-me]

I made this mistake a few months ago, luckily someone not experienced than me, fixed kit for me :)

[alamb]

@jorgecarleitao no worries -- I live in constant fear I will do something like this too... :)