diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index c6fb1cd9c..e4538955c 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -1033,6 +1033,21 @@ static ArrowErrorCode ArrowArrayFinalizeBuffers(struct ArrowArray* array) { } struct ArrowBuffer* buffer = ArrowArrayBuffer(array, i); + + // An offsets buffer holds length + 1 offsets even for an empty array, and the C + // data interface carries no buffer sizes, so consumers read offsets[0] + // unconditionally. IPC permits a zero-length offsets buffer for an empty array + // (nanoarrow's encoder writes one), which would otherwise be exported as a pointer + // to no valid bytes (or into a neighbouring part of a shared message body). + if (private_data->layout.buffer_type[i] == NANOARROW_BUFFER_TYPE_DATA_OFFSET && + array->length == 0 && buffer->size_bytes == 0) { + ArrowBufferReset(buffer); + int64_t zero = 0; + NANOARROW_RETURN_NOT_OK(ArrowBufferAppend( + buffer, &zero, private_data->layout.element_size_bits[i] / 8)); + continue; + } + if (buffer->data == NULL) { NANOARROW_RETURN_NOT_OK((ArrowBufferReserve(buffer, 1))); } diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc index cd9ac88e1..9324cdde6 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -383,6 +383,59 @@ TEST(ArrayTest, ArrayTestBuildByBuffer) { ArrowArrayRelease(&array); } +TEST(ArrayTest, ArrayTestFinishBuildingEmptyArrayOffsets) { + // An empty variable-size array still has one offset, and the C data interface carries + // no buffer sizes, so consumers read offsets[0] unconditionally. Finishing an empty + // array whose offsets buffer holds no bytes must leave exactly one zero offset there, + // whether that buffer was never allocated or points at bytes that aren't offsets. + struct ArrowError error; + struct TestCase { + enum ArrowType type; + int64_t offset_size; + bool garbage_storage; + }; + + for (const auto& test_case : {TestCase{NANOARROW_TYPE_STRING, 4, false}, + TestCase{NANOARROW_TYPE_STRING, 4, true}, + TestCase{NANOARROW_TYPE_LARGE_BINARY, 8, false}, + TestCase{NANOARROW_TYPE_LARGE_BINARY, 8, true}, + TestCase{NANOARROW_TYPE_LIST, 4, false}, + TestCase{NANOARROW_TYPE_LIST, 4, true}}) { + SCOPED_TRACE(std::string(ArrowTypeString(test_case.type)) + + (test_case.garbage_storage ? " with garbage storage" : "")); + + nanoarrow::UniqueSchema schema; + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), test_case.type), NANOARROW_OK); + if (test_case.type == NANOARROW_TYPE_LIST) { + ASSERT_EQ(ArrowSchemaSetType(schema->children[0], NANOARROW_TYPE_INT32), + NANOARROW_OK); + } + + nanoarrow::UniqueArray array; + ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), &error), NANOARROW_OK) + << error.message; + struct ArrowBuffer* offsets = ArrowArrayBuffer(array.get(), 1); + ASSERT_EQ(offsets->size_bytes, 0); + if (test_case.garbage_storage) { + // Storage behind a zero-length buffer, as when an empty array's offsets buffer + // points into some other part of a larger allocation + ASSERT_EQ(ArrowBufferReserve(offsets, 16), NANOARROW_OK); + memset(offsets->data, 0xff, 16); + } + + ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), &error), NANOARROW_OK) + << error.message; + ASSERT_NE(array->buffers[1], nullptr); + EXPECT_EQ(ArrowArrayBuffer(array.get(), 1)->size_bytes, test_case.offset_size); + int64_t first_offset = -1; + memcpy(&first_offset, array->buffers[1], test_case.offset_size); + if (test_case.offset_size == 4) { + first_offset = static_cast(first_offset); + } + EXPECT_EQ(first_offset, 0); + } +} + TEST(ArrayTest, ArrayTestExplicitValidationLevel) { struct ArrowArray array; struct ArrowError error; diff --git a/src/nanoarrow/ipc/decoder_test.cc b/src/nanoarrow/ipc/decoder_test.cc index c7ac78ce5..24acafe64 100644 --- a/src/nanoarrow/ipc/decoder_test.cc +++ b/src/nanoarrow/ipc/decoder_test.cc @@ -122,6 +122,31 @@ alignas(8) static uint8_t kSimpleRecordBatch[] = { 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; +// One row of struct, i: int32> (l = [[]], i = 0x55555555) as written by +// nanoarrow's encoder. The empty string array's offsets buffer has zero bytes and the +// same body offset as i's data buffer. +alignas(8) static uint8_t kEmptyChildRecordBatch[] = { + 0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x1a, 0xff, + 0xff, 0xff, 0x04, 0x00, 0x03, 0x00, 0x10, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3c, 0xff, 0xff, 0xff, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x84, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x14, 0x00, 0x04, 0x00, + 0x0c, 0x00, 0x10, 0x00, 0x0c, 0x00, 0x14, 0x00, 0x04, 0x00, 0x06, 0x00, 0x08, 0x00, + 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0x55, 0x55, 0x55, 0x00, 0x00, 0x00, 0x00}; + alignas(8) static uint8_t kSimpleRecordBatchCompressedZstd[] = { 0xff, 0xff, 0xff, 0xff, 0xa0, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x18, 0x00, 0x06, 0x00, 0x05, 0x00, 0x08, 0x00, 0x0c, 0x00, @@ -2691,3 +2716,61 @@ INSTANTIATE_TEST_SUITE_P(NanoarrowIpcDecoder, DeltaDictionaryTypeTest, NANOARROW_TYPE_STRUCT, NANOARROW_TYPE_FIXED_SIZE_LIST)); #endif + +TEST(NanoarrowIpcTest, NanoarrowIpcDecodeEmptyArrayOffsets) { + // An empty array whose offsets buffer holds no bytes must still be decoded with a + // readable zero offset, whether or not the decoded array shares the message body + struct ArrowError error; + + nanoarrow::UniqueSchema schema; + ArrowSchemaInit(schema.get()); + ASSERT_EQ(ArrowSchemaSetTypeStruct(schema.get(), 2), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetType(schema->children[0], NANOARROW_TYPE_LIST), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetType(schema->children[0]->children[0], NANOARROW_TYPE_STRING), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetType(schema->children[1], NANOARROW_TYPE_INT32), NANOARROW_OK); + + nanoarrow::ipc::UniqueDecoder decoder; + ASSERT_EQ(ArrowIpcDecoderInit(decoder.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcDecoderSetSchema(decoder.get(), schema.get(), &error), NANOARROW_OK) + << error.message; + + struct ArrowBufferView data; + data.data.as_uint8 = kEmptyChildRecordBatch; + data.size_bytes = sizeof(kEmptyChildRecordBatch); + ASSERT_EQ(ArrowIpcDecoderDecodeHeader(decoder.get(), data, &error), NANOARROW_OK) + << error.message; + struct ArrowBufferView body; + body.data.as_uint8 = kEmptyChildRecordBatch + decoder->header_size_bytes; + body.size_bytes = decoder->body_size_bytes; + + for (bool shared : {false, true}) { + SCOPED_TRACE(shared ? "shared body" : "borrowed body"); + nanoarrow::UniqueArray array; + if (shared) { + nanoarrow::UniqueBuffer body_copy; + ASSERT_EQ(ArrowBufferAppend(body_copy.get(), body.data.data, body.size_bytes), + NANOARROW_OK); + nanoarrow::UniqueBuffer shared_body; + ASSERT_EQ(ArrowSharedBufferInit(shared_body.get(), body_copy.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcDecoderDecodeArrayFromShared( + decoder.get(), shared_body.get(), -1, array.get(), + NANOARROW_VALIDATION_LEVEL_FULL, &error), + NANOARROW_OK) + << error.message; + } else { + ASSERT_EQ(ArrowIpcDecoderDecodeArray(decoder.get(), body, -1, array.get(), + NANOARROW_VALIDATION_LEVEL_FULL, &error), + NANOARROW_OK) + << error.message; + } + + ASSERT_EQ(array->length, 1); + struct ArrowArray* strings = array->children[0]->children[0]; + ASSERT_EQ(strings->length, 0); + EXPECT_EQ(ArrowArrayBuffer(strings, 1)->size_bytes, 4); + int32_t first_offset = -1; + memcpy(&first_offset, strings->buffers[1], sizeof(first_offset)); + EXPECT_EQ(first_offset, 0); + } +}