Add BooleanArray::new_from_packed and BooleanArray::new_from_u8

[chloro-pn]

Which issue does this PR close?

Closes #.

Rationale for this change

Users can easily build BooleanArray from &[u8]

What changes are included in this PR?

• add BooleanArray::new_from_packed
• add BooleanArray::new_from_u8

Are there any user-facing changes?

[tustvold]

I'm not sure about this, as it doesn't provide a mechanism to indicate a length that isn't a multiple of 8 or aligned.

Perhaps you could expand on what the use-case for this is, and why you would be using this as opposed to say BooleanBufferBuilder or similar?

[chloro-pn]

I'm not sure about this, as it doesn't provide a mechanism to indicate a length that isn't a multiple of 8 or aligned.

Perhaps you could expand on what the use-case for this is, and why you would be using this as opposed to say BooleanBufferBuilder or similar?

Here, we only convert each bit in &[u8] to boolean and use it to build BooleanArray. It does not involve directly setting certain internal components, so there is no need to consider alignment and whether the length is a multiple of 8.
For example, in some scenarios, we need to use a bitstream (represented as &[u8] in Rust) to construct objects of type BooleanArray, where each bit in the bitstream is 1 for true and 0 for false.
If we didn't have this PR, we would always need to:

• first build Buffer from &[u8],
• then build BooleanBuffer from Buffer, and
• finally build BooleanArray from BooleanBuffer.

Alternatively, we need to convert the bitstream into an iterator object that returns a boolean, but these come at a performance loss.

[chloro-pn]

Of course, using this method involves two self-evident facts:

• There is no null in the constructed BooleanArray
• The length of the constructed BooleanArray is always a multiple of 8;

If these prerequisites are not met, then this method should not be used.
But these will not cause confusion or difficulty for users to understand, as the mapping from bit stream to boolean array is intuitive.

[alamb]

I wonder if this related to the 8 bit boolean extension type discussion on the dev list: https://lists.apache.org/thread/nz44qllq53h6kjl3rhy0531n2n2tpfr0 ?

[alamb]

Thanks for this proposal @chloro-pn

[chloro-pn]

I wonder if this related to the 8 bit boolean extension type discussion on the dev list: https://lists.apache.org/thread/nz44qllq53h6kjl3rhy0531n2n2tpfr0 ?

No, we are not building a new type here.
We are just providing a more convenient way to build BooleanArray from bitstream, which will have better performance and be more convenient under the premise of meeting usage requirements.

[alamb]

I thought about this more last night.

I think my main concern is if the semantics of BooleanArray::from(&[u8]) will be "surprising" to people

Thus I would like to propose making this a function like BooleanArray::from_u8 like

let arr = BooleanArray::from_u8(..)

We can put the documentation on that method. I think that would avoid any concern about confusion (as users would could the method explicitly).

What do you think?

[alamb]

Thank you.

[chloro-pn]

I thought about this more last night.

I think my main concern is if the semantics of BooleanArray::from(&[u8]) will be "surprising" to people

Thus I would like to propose making this a function like BooleanArray::from_u8 like

let arr = BooleanArray::from_u8(..)

We can put the documentation on that method. I think that would avoid any concern about confusion (as users would could the method explicitly).

What do you think?

I think your suggestion is more appropriate, and I will modify the code later.
The use of &[u8] to represent a bitstream is not so obvious and consistent (&[u8] can only represent a partial bitstream).
The From trait is not only used to represent conversions between types, but also declares a semantically equivalent relationship.I get it.
I am a beginner in Rust, thanks for your code review: )

[tustvold]

Might I suggest something along the lines of

fn new_from_packed(buffer: impl Into<Buffer>, offset: usize, len:usize)

This would be explicit and consistent with other methods.

Edit: although I wonder if this is then really all that different from

let a: BooleanArray = BooleanBuffer(b.into(), 0, len).into();

I wonder if we really need a new method...

[chloro-pn]

Might I suggest something along the lines of

fn new_from_packed(buffer: impl Into<Buffer>, offset: usize, len:usize)

This would be explicit and consistent with other methods.

Edit: although I wonder if this is then really all that different from

let a: BooleanArray = BooleanBuffer(b.into(), 0, len).into();

I wonder if we really need a new method...

This is what I mentioned in my previous reply "providing a more convenient way to build BooleanArray from bitstream".
As a developer who is very familiar with arrow-rs, this method is unnecessary, but for users, I think the provided interface should try to hide the internal structure as much as possible and maintain simplicity.

new_from_packed is a good idea, perhaps we should implement this more universal version.

[tustvold]

My feeling is this convenience comes at the expense of being rather confusing to users, especially those less familiar with the internal layout of arrow arrays. My 2 cents is if people want a convenient and easy interface they can use the builders or iterators, if they want lower level integration they use lower level interfaces and construct the array from parts, providing a method that is convenient but correct usage requires knowledge of the internal layout seems undesirable.

If Andrew is happy to move forward with this I have no objection to this, but I'm not sold

[alamb]

I agree that it takes a lot of knowledge until this is obvious:

let a: BooleanArray = BooleanBuffer(b.into(), 0, len).into();

It seems to me adding an example of how to do this in the documentation is valuable in any case.

I also think it is valuable to add APIs that make arrow-rs easier to use for less expert users, even if that means they aren't the most concise, and thus I think an API such as proposd in the PR is a good addition

[alamb]

Thank you @chloro-pn -- I think this is a nice addition to the API that is well commented and explains what it does well.

[alamb]

❤️

[chloro-pn]

I think many people have said it before, but I still want to say that you are a very nice person. Thank you : )

[alamb]

It is all part of my master plan to get everyone working together well to build this ecosystem together -- it is great fun.

[alamb]

Thank you for the review and comments @tustvold

[alamb]

Thanks again @chloro-pn and @tustvold