rustyconover opened a new pull request, #966: URL: https://github.com/apache/arrow-nanoarrow/pull/966
An empty variable-size array (string, binary, their large variants, list, large list, map) needs one offset, `offsets[0]`, even though it has no elements. The C data interface doesn't carry buffer sizes, so consumers read `offsets[0]` without checking. Arrow C++'s `ImportArray()` does this, for example. IPC allows a zero-length offsets buffer for an empty array, and nanoarrow's own encoder writes one, since `ArrowArrayViewSetArray()` sizes the offsets buffer of a length-0 array as 0 bytes. When such an array is decoded, `ArrowArrayFinishBuilding()` leaves the empty buffer as it is. If the buffer is NULL, it reserves one byte but never initializes it. Either way, the exported `ArrowArray` has an offsets pointer with no valid offset behind it. With shared buffers, that pointer points into whatever follows in the message body. I found this moving a DuckDB extension's IPC from Arrow C++ to nanoarrow. A `MAP` column holding a single `NULL` round-trips through a worker as an empty key/value child. After nanoarrow decoded it and Arrow C++ imported it, about one run in four failed `ValidateFull()` with `array starts at negative offset -2099303496`. The value depends on whatever bytes happen to sit in the message body after the empty buffer. With this PR, `ArrowArrayFinalizeBuffers()` gives a `DATA_OFFSET` buffer of an empty array, if it holds no bytes, a single zero offset of the right width (4 or 8 bytes). This works for both owned and shared buffers: the shared reference is released, and the buffer is replaced with an owned one. Arrays with elements are left alone, so validation still reports a truncated offsets buffer as before (`ArrayTestExplicitValidationLevel` checks this). Things worth a look in review: - `ArrowArrayFinalizeBuffers()` only runs at `NANOARROW_VALIDATION_LEVEL_DEFAULT` or higher, because it assumes CPU data access. Decoding at `MINIMAL` or `NONE` still exports the empty buffer unchanged. I've left that gate alone. Putting a host buffer behind a device array would be worse. - An alternative would be for the encoder to always write `offsets[0]`, as Arrow C++ does. That only covers streams nanoarrow wrote itself, though. Other writers can emit zero-length offsets buffers too, so the fix belongs on the decode side. Tests: `NanoarrowIpcWriter.RoundtripEmptyArraysHaveZeroOffset` writes a zero-row batch with `string`, `large_binary` and `list<int32>` columns. It reads the batch back with and without shared buffers, then checks that each exported offsets buffer has a size of 4 or 8 bytes and holds 0. The test fails on `main`, where the buffers are 0 bytes. ctest passes locally (macOS arm64, Debug, 389 tests). The touched suites (array, IPC writer/reader/decoder) pass under ASan/UBSan. `array_test` needs `allocator_may_return_null=1` there, as it does on `main`. 🤖 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]
