Support Utf8View for bit_length kernel

[austin362667]

Which issue does this PR close?

Closes apache/datafusion#13195

Rationale for this change

Thanks to @jayzhan211 , he noticed following issue, array compute kernel bit_length() doesn't support Utf8View type:

create table test_source as values
  ('Andrew', 'X'),
  ('Xiangpeng', 'Xiangpeng'),
  ('Raphael', 'R'),
  (NULL, 'R');
create table test as
SELECT
  arrow_cast(column1, 'Utf8View') as column1_utf8view
FROM test_source;

select bit_length(column1_utf8view) from test;

caused the error:

query error DataFusion error: Arrow error: Compute error: bit_length not supported for Utf8View

What changes are included in this PR?

Update bit_length() array function to support Utf8View

Are there any user-facing changes?

[tustvold]

I think this needs a test, which I think will show this not working

[alamb]

Thanks @austin362667 -- this is looking good. I started the CI and left some comments

[alamb]

Thanks @austin362667

[alamb]

I think the length is currently treated as a u32 elsewhere in the code

However the actual Arrow spec I think says use i32 so this is probably fine

[alamb]

I also still do think you need to handle the null cases (check for which slots are null) as there might be a view of a non zero length there

[austin362667]

Sure! Thanks @alamb for review

[findepi]

In #6662 (comment) it was suggested to use from_unary.
It this applicable here?

[austin362667]

@findepi Thanks for bringing this up. I tried it and it works trivially well.

[tustvold]

Using from_unary reverts the optimisation of only inspecting the views, and not the string data.

Perhaps you might use the example I wrote in #6671 (comment)

[findepi]

I agree that bit_length should not beed to return signed integer, but it seems better not to change this within this PR.

[tustvold]

Suggested change

	let values = (0..list.len())
	.map(|i| {
	if list.is_valid(i) {
	list.views()[i] as u32 * 8
	} else {
	0
	}
	})
	.collect::<Vec<u32>>();
	Ok(Arc::new(UInt32Array::new(
	values.into(),
	let values = list.views().iter().map(|view| (view as i32).wrapping_mul(8)).collect();
	Ok(Arc::new(Int32Array::new(
	values,

As we're doing infallible wrapping arithmetic, we can just compute this for everything, this will vectorize better, and the null mask is preserved below

[austin362667]

Wow! Thank you so much @tustvold , you are indeed making me a better Rust programmer. Really appreciate for review. 👍

[tustvold]

I think this can be further simplified, I also think we should return Int32Array for consistency with the other implementations.

[austin362667]

Thanks @tustvold @alamb @findepi for the review.

• I roll back to i32 rather than u32 to make code consistent.
• Use from_unary suggested from @findepi
• Add bit_length_null_utf8view test cases suggested by @tustvold .

[tustvold]

Switching to from_unary has reverted the optimisation to not inspect the string data - #6671 (comment) is how to do this correctly

[austin362667]

Got it! Thanks @tustvold I replaced from_unary with the approach you suggested.

[alamb]

Looks great to me -- thank you @austin362667 and @tustvold and @findepi