Add NullBuffer::try_from_unsliced helper and refactor call sites

[Eyad3skr]

Implements a helper to replace the pattern of creating a BooleanBuffer from an unsliced validity bitmap and filtering by null count. Previously this was done with BooleanBuffer::new(...) plus Some(NullBuffer::new(...)).filter(|n| n.null_count() > 0); now it is a single call to NullBuffer::try_from_unsliced(buffer, len), which returns Some(NullBuffer) when there are nulls and None when all values are valid.

• Added try_from_unsliced in arrow-buffer/src/buffer/null.rs with tests for nulls, all valid, all null, empty
• Refactor FixedSizeBinaryArray::try_from_iter_with_size and try_from_sparse_iter_with_size to use it
• Refactor take_nulls in arrow-select to use it

Closes #9385

[alamb]

Thank you @Eyad3skr -- this looks nice to me

cc @liamzwbao in case you have time to help reivew

[alamb]

Could you also update this comment to reflect the offset is in bits (not bytes)? Mixing the units is a common mistake so making sure the documentation is as clear as possible would help

[Jefffrey]

Maybe we should call it from_buffer? try_* probably should be reserved for functions that return a result, and its probably better to be direct that we're constructing directly from a buffer?

[Eyad3skr]

a valid point tbh! on it.

[Eyad3skr]

@alamb I guess there is nothing else to take care after anymore? maybe if someone can just trigger/approve the CI pipeline workflows left that would be awesome.

[alamb]

@alamb I guess there is nothing else to take care after anymore? maybe if someone can just trigger/approve the CI pipeline workflows left that would be awesome.

Done!

[alamb]

Seems like the CI is broken

[Eyad3skr]

true, my schedule this week is a bit busy because of university. I will be working on fixing the CI between today and Friday. On it.

[liamzwbao]

Overall LGTM! thanks for the change.

It would be better to refactor the following call sites as well

• arrow-rs/arrow-array/src/array/boolean_array.rs

  Lines 537 to 542 in c129c7c

  	let nulls = Some(NullBuffer::new(BooleanBuffer::new(
  	null_builder.into(),
  	0,
  	data_len,
  	)))
  	.filter(|n| n.null_count() > 0);

• arrow-rs/arrow-string/src/regexp.rs

  Lines 203 to 207 in c129c7c

  	let nulls = array
  	.nulls()
  	.map(|n| n.inner().sliced())
  	.map(|b| NullBuffer::new(BooleanBuffer::new(b, 0, array.len())))
  	.filter(|n| n.null_count() > 0);

• arrow-rs/arrow-string/src/substring.rs

  Lines 216 to 220 in c129c7c

  	let nulls = array
  	.nulls()
  	.map(|n| n.inner().sliced())
  	.map(|b| NullBuffer::new(BooleanBuffer::new(b, 0, array.len())))
  	.filter(|n| n.null_count() > 0);

• arrow-rs/arrow-string/src/substring.rs

  Lines 318 to 322 in c129c7c

  	let nulls = array
  	.nulls()
  	.map(|n| n.inner().sliced())
  	.map(|b| NullBuffer::new(BooleanBuffer::new(b, 0, array.len())))
  	.filter(|n| n.null_count() > 0);

• arrow-rs/arrow-string/src/substring.rs

  Lines 356 to 360 in c129c7c

  	let mut nulls = array
  	.nulls()
  	.map(|n| n.inner().sliced())
  	.map(|b| NullBuffer::new(BooleanBuffer::new(b, 0, num_of_elements)))
  	.filter(|n| n.null_count() > 0);

[liamzwbao]

Arrow uses LSB numbering and that's why this test failed. Could refer to the doc to fix the test