Check the length of null_bit_buffer in ArrayData::try_new()

[HaoYang670]

Signed-off-by: remzi 13716567376yh@gmail.com

Which issue does this PR close?

Closes #1707.

Rationale for this change

The ArrayData::try_new will panic if null_bit_buffer is too short.

What changes are included in this PR?

Check the length of null_bit_buffer before we call new_unchecked in ArrayData::try_new()

Are there any user-facing changes?

No.

[HaoYang670]

I am not sure if this can be removed: https://github.com/apache/arrow-rs/blob/master/arrow/src/array/data.rs#L683-L693
as validate() is a public function.

But if we don't, we will check the length of null_bit_buffer twice.
Have no good idea on it.

[tustvold]

LGTM, thank you 👍

[HaoYang670]

But if we don't, we will check the length of null_bit_buffer twice.

Maybe in the long term, we should try to remove the null_count field from ArrayData. I don't find why we need this field except making fn null_count() as a const function (Who will call null_count() multiple times on same array?).

[tustvold]

we should try to remove the null_count field from ArrayData

Often faster kernel implementations are possible if one can ignore nulls, as such null_count helps to inform this selection. It is also fairly common for a given array to be fed to more than one kernel, e.g. cmp then filter, and as such caching the null count on ArrayData avoids computing it multiple times

[HaoYang670]

Thank you for your explanation @tustvold. I will do some experiments to test how much performance null_count could contribute.

[viirya]

Looks good. Thanks @HaoYang670