hcrosse opened a new pull request, #1461: URL: https://github.com/apache/arrow-go/pull/1461
### Rationale for this change `is_null` and `is_not_null` read only a dictionary array's index validity, so a valid index pointing to a null value counts as not null. With values `["a", null]` and indices `[0, 1, null]`, `is_null` returns `[false false true]` instead of `[false true true]`. Arrow C++ treats that second slot as null. pyarrow `26.0.1` returns `[False, True, True]` for `is_null` and `[True, False, False]` for `is_valid`, the counterpart of `is_not_null`. ### What changes are included in this PR? When dictionary values may have nulls, both kernels build a bitmap from the index validity and the referenced value's validity, checking the index first, as C++'s [`dict_util::SetLogicalNullBits`](https://github.com/apache/arrow/blob/0ab78e2df489055b71a7d26a666a52823f87f01c/cpp/src/arrow/util/dict_util.cc#L122) does. A null slot can hold an index outside the dictionary, so its value must not be read. Other inputs keep the existing path. For dictionaries without null values, `is_not_null` still shares the index validity buffer at offset 0. Out of scope: - `nan_is_null`, which the kernel does not read. - Extension types with dictionary storage. - Dictionaries whose values are union or run-end encoded, which C++ also does not handle. - Dictionaries whose values are a null type, which C++ reports as null and this change still reports as valid. - Null, union and run-end encoded arrays themselves, where `is_null` also disagrees with C++ and each type needs its own handling. ### Are these changes tested? I added tests for every integer index type, an index outside the dictionary in a null slot, and indices without a validity buffer. They also cover index and dictionary offsets crossing a byte boundary, scalar and chunked inputs, and the unchanged fast path, including buffer sharing at offset 0. The tests for null dictionary values fail without the fix. On macOS arm64, `go test ./arrow/compute/...` passed with and without `-tags assert,test`, along with `go test -race ./arrow/compute/`, `go vet`, `gofmt`, golangci-lint and the prek Release Audit Tool. https://github.com/apache/arrow-go/pull/1457 also changes how these kernels are registered. This change merges cleanly with it, and the compute tests pass on the merged tree. ### Are there any user-facing changes? Yes. `is_null` and `is_not_null` now count a valid index pointing to a null dictionary value as null. There is no API change. Closes https://github.com/apache/arrow-go/issues/1452. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
