This is the core validation check.

These tests need to be updated because they were creating invalid Buffers (this one for example had an len of 5 and offset of 2 but only passed in an array 5 long)

the array data in this case is only 4 elements long, so using an offset of 1 with len 4 is incorrect (goes off the end of the array data). The same with the test below

build() now fails the earlier validation check -- so to keep the test checking the internal BooleanArray checks need to use unsafe

Note that it might be best to eventually remove all Array specific checks (which I think will be redundant) in favor of consolidated checks in ArrayData.rs but I don't want to consider doing that until I have the validation checks completed

This is a similar invalid test that the new checks identified -- value_data has only three items in it, so doing offset = 1 and len = 3 can potentially read off the end. This change corrects the len to be within bounds.

There is a very similar (and thus fixed) test in array_list.rs and one in array_map.rs

this test is checking the ListArray specific error, but since the new ArrayData validation checks now catch the problem we need to switch to using build_unchecked() to exercise the same code path

The "safe" way here would be to fail validation always for Union to signal to the user that the validation checks are not correct.

I think a better story would be to fix #85 and #814 . I am torn on what to do in this case

I finally settled on leaving UnionArray unvalidated for this PR so that I can backport it to the 6.x release line (it is backwards compatible) and in a PR that is backward incompatible I will fixup the UnionArray implementation (and validation)

As part of #85 I will clean up the union array validation

I'm wondering what could happen if the null_count actually gives a different number than the validity buffer and whether this could lead to undefined behavior in some later operation. Most callers pass None anyway, so we calculate a number which is guaranteed to be in range. I would suggest to remove the null_count parameter from try_new to completely avoid of inconsistencies.

Kernels that want to avoid the overhead of counting bits could use the unsafe new_unchecked method. I see it is currently set by the cast_array_data function. To make that one safe while avoiding bit counting, we could just validate that the from and to layouts are equal.

I'm wondering what could happen if the null_count actually gives a different number than the validity buffer and whether this could lead to undefined behavior in some later operation. Most callers pass None anyway, so we calculate a number which is guaranteed to be in range.

I suspect (but can not prove) that if null_count is inconsistent with the validity buffer then there may be incorrect answers but not undefined behavior (in the Rust sense).

My plan will be:

1. In a separate PR (as it will not be backwards compatible) remove the null_count from try_new(): Remove null_count from ArrayData::try_new() #911
2. in the PR where I add the full on data checking (validate_full()) ensure that the declared null count matches the validity bitmap

This seems to be only used for validating dictionary key types, maybe rename to is_dictionary_key_type and link the ArrowDictionaryKeyType trait in the comment.

Done in 8acd8d2