rustyconover opened a new pull request, #974: URL: https://github.com/apache/arrow-nanoarrow/pull/974
`ArrowIpcEncoderCollectArray()` copied `ArrowArrayView.null_count` into each `FieldNode` as it was. That is wrong in two cases: - **Unknown null count.** In the C data interface, `null_count` may be -1 when it hasn't been computed. A zero-copy slice often looks like this. The IPC format has no such value: `Message.fbs` documents `FieldNode.null_count` as "the number of observed nulls", and `Columnar.rst` treats the null count as a property of the array that is "as large as the array length". The encoder wrote `null_count: -1`. - **Null type.** Every slot of a null array is null, so its null count is its length. Arrow C++ enforces this (`ValidateArray` reports "Null array null_count unequal to its length", and `NullArray` forces `null_count = length`). A producer may still leave a null array's `null_count` at 0, and the encoder wrote that 0. The encoder now writes the length for a null array. For other types it computes the null count with `ArrowArrayViewComputeNullCount()` when the count is negative. The change is in `ArrowIpcEncoderCollectArray()`, which also collects dictionary values, so it applies to `DictionaryBatch` messages as well. The top-level record batch check already handled -1. I found this when a DuckDB extension started writing its IPC with nanoarrow and Apache.Arrow for .NET read the stream. Arrow C++ happens to accept both cases: it maps -1 to `kUnknownNullCount` and recomputes it, and `NullArray` overwrites the count. Apache.Arrow 23.0.0 (.NET) does not: - `null_count: -1` throws `InvalidDataException: Null count of -1 must be >= 0 and <= 2147483647`. Older versions read the column as having no nulls, so NULLs were silently lost. - A null child with `null_count: 0` throws `ArgumentException: Length must equal null count` from `NullArray..ctor`. With this change, both batches read correctly in .NET. Tests: `NanoarrowIpcEncoderWritesActualNullCounts` encodes `struct<i: int32, n: null>` with `i = [1, null, 3]`, `i.null_count = -1` and `n.null_count = 0`. It decodes the message and checks the field-node null counts, which must be 1 and 3. On `main` the test fails with -1 and 0. `ctest` passes (Linux aarch64, Debug, 393 tests), including under ASan/UBSan with clang. The changed files are formatted with clang-format 16.0.6, the pre-commit version. This PR's code and description were written by AI (Claude Code) and reviewed by a human maintainer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
