hcrosse opened a new pull request, #1488:
URL: https://github.com/apache/arrow-go/pull/1488
### Rationale for this change
`is_null` accepts `NullOptions.NanIsNull` but ignores it, so NaN is never
reported as null.
### What changes are included in this PR?
With `NanIsNull` set, `is_null` reports NaN as null for `float16`, `float32`
and `float64` arrays and scalars, and for dictionaries with these value types.
For dictionaries, the value validity and NaN bits are combined in a temporary
bitmap, and the existing dictionary loop follows the indices into it.
`is_null` now has a kernel initializer that defaults to `NullOptions{}`. It
accepts nil, a `NullOptions` value or a pointer, including a nil pointer. Nil
is needed because `exprs` initializes kernels without applying function
defaults. Options of another type return `arrow.ErrInvalid`, as in the other
kernels with options.
`NullOptions` moves into the internal kernels package so the kernel can read
it, and `compute.NullOptions` becomes an alias, as `CastOptions` did in
https://github.com/apache/arrow-go/commit/14455e6bd1fde0a02cc4978f4b9cf6dda878b986.
Composite literals, field access and serialization keep working, but the
reflected type name and package path change. `apidiff` reports:
```text
Incompatible changes:
- NullOptions: changed from NullOptions to
github.com/apache/arrow-go/v18/arrow/compute/internal/kernels.NullOptions
```
`is_not_null` is unchanged. Union, run-end encoded and extension inputs
don't check for NaN, as in Arrow C++.
### Are these changes tested?
Yes. I added three tests to `ScalarValiditySuite`, which uses a checked
allocator:
- `TestIsNullNanIsNull` covers arrays and scalars of all three float widths
with finite values, signed zeros, infinities, nulls and NaN, each option form,
and a slice at offset 9.
- `TestIsNullNanIsNullDictionary` covers null indices, null and NaN values,
values without a validity bitmap, and sliced indices and values. Each case also
runs as scalars.
- `TestIsNullInvalidOptions` checks that another options type returns
`arrow.ErrInvalid`.
The NaN cases and the options type case fail on main and pass with this
change. The issue's reproduction prints true at each NaN position with this
change.
On macOS arm64 with Go 1.26.10, `go test ./arrow/...` passed with
arrow-testing and parquet-testing checked out. `go test -race -tags
assert,test` and `go vet` on `arrow/compute/...`, `gofmt`, `golangci-lint`
2.13.2 in `arrow`, `parquet` and `internal`, and the Release Audit Tool also
passed.
### Are there any user-facing changes?
Yes. `NanIsNull` now affects floating point inputs, and `is_null` rejects
options of another type. `compute.NullOptions` becomes an alias of an internal
type, so its reflected type name and package path change.
Closes https://github.com/apache/arrow-go/issues/1484.
--
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]
