From d8b952f549a7100d4196a4a8e16328b1d45f5f5b Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 19:09:55 -0400 Subject: [PATCH 01/16] fix(ipc): attach metadata to dictionary batches --- src/nanoarrow/ipc/encoder.c | 3 +++ src/nanoarrow/ipc/encoder_test.cc | 24 ++++++++++++++++++++++++ src/nanoarrow/nanoarrow_ipc.h | 7 ++++--- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/nanoarrow/ipc/encoder.c b/src/nanoarrow/ipc/encoder.c index ab7f0b60d..ffc9d6f8e 100644 --- a/src/nanoarrow/ipc/encoder.c +++ b/src/nanoarrow/ipc/encoder.c @@ -1124,6 +1124,9 @@ static ArrowErrorCode ArrowIpcEncoderEncodeDictionaryBatch( FLATCC_RETURN_UNLESS_0(DictionaryBatch_data_end(builder), error); FLATCC_RETURN_UNLESS_0(DictionaryBatch_isDelta_add(builder, is_delta ? 1 : 0), error); FLATCC_RETURN_UNLESS_0(Message_header_DictionaryBatch_end(builder), error); + + NANOARROW_RETURN_NOT_OK(ArrowIpcEncodeMessageMetadata(private, error)); + FLATCC_RETURN_UNLESS_0(Message_bodyLength_add(builder, buffer_encoder->body_length), error); FLATCC_RETURN_IF_NULL(ns(Message_end_as_root(builder)), error); diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index 00fd278e5..6457e09ea 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -468,6 +468,8 @@ TEST(NanoarrowIpcTest, NanoarrowIpcVisitMessageMetadataError) { TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryBatch) { nanoarrow::ipc::UniqueEncoder encoder; ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueDecoder decoder; + ASSERT_EQ(ArrowIpcDecoderInit(decoder.get()), NANOARROW_OK); // Build a simple Utf8 values array nanoarrow::UniqueSchema values_schema; @@ -495,6 +497,13 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryBatch) { NANOARROW_OK) << error.message; + KeyValues message_key_values{{"dictionary_key", "dictionary_value"}}; + auto message_metadata = PackMetadata(message_key_values); + ASSERT_EQ( + ArrowIpcEncoderSetMessageMetadata(encoder.get(), message_metadata.get(), &error), + NANOARROW_OK) + << error.message; + // Encode a non-delta DictionaryBatch with dictionary_id=0 nanoarrow::UniqueBuffer body_buffer; EXPECT_EQ(ArrowIpcEncoderEncodeSimpleDictionaryBatch(encoder.get(), /*dictionary_id=*/0, @@ -511,6 +520,21 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryBatch) { // The encapsulated message must be non-empty and 8-byte aligned EXPECT_GT(message_buffer->size_bytes, 8); EXPECT_EQ(message_buffer->size_bytes % 8, 0); + EXPECT_EQ(DecodeMessageMetadata(message_buffer.get(), decoder.get()), + message_key_values); + + // The metadata applies to exactly one message: the next DictionaryBatch has none + message_buffer->size_bytes = 0; + body_buffer->size_bytes = 0; + ASSERT_EQ(ArrowIpcEncoderEncodeSimpleDictionaryBatch(encoder.get(), /*dictionary_id=*/0, + /*is_delta=*/0, values_view.get(), + body_buffer.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/1, + message_buffer.get()), + NANOARROW_OK); + EXPECT_EQ(DecodeMessageMetadata(message_buffer.get(), decoder.get()), KeyValues{}); } // A record batch whose columns exercise each path of the compressed body builder: diff --git a/src/nanoarrow/nanoarrow_ipc.h b/src/nanoarrow/nanoarrow_ipc.h index 7f5559759..87ee87cc6 100644 --- a/src/nanoarrow/nanoarrow_ipc.h +++ b/src/nanoarrow/nanoarrow_ipc.h @@ -979,9 +979,10 @@ NANOARROW_DLL ArrowErrorCode ArrowIpcEncoderFinalizeBuffer( /// \brief Set the custom metadata of the next encoded message /// -/// Attaches metadata to the next message encoded by ArrowIpcEncoderEncodeSchema() or -/// ArrowIpcEncoderEncodeSimpleRecordBatch() (i.e., Message::custom_metadata, which is -/// distinct from the metadata of the Schema or Field that the message may contain). +/// Attaches metadata to the next message encoded by ArrowIpcEncoderEncodeSchema(), +/// ArrowIpcEncoderEncodeSimpleRecordBatch(), or +/// ArrowIpcEncoderEncodeSimpleDictionaryBatch() (i.e., Message::custom_metadata, which +/// is distinct from the metadata of the Schema or Field that the message may contain). /// The metadata applies to exactly one message: after a message is encoded the /// encoder's message metadata is cleared. Any metadata that was set but not yet /// encoded is replaced by this call; pass NULL to clear it. From 7855e91e7e634e1115efcf17220b502b09d953b2 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 19:22:04 -0400 Subject: [PATCH 02/16] fix(ipc): avoid repeating unchanged dictionaries --- src/nanoarrow/ipc/writer.c | 180 +++++++++++++++++++++++++------ src/nanoarrow/ipc/writer_test.cc | 113 ++++++++++++++++++- 2 files changed, 257 insertions(+), 36 deletions(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index bb8c79710..6d9c1c31b 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -181,8 +181,30 @@ struct ArrowIpcWriterPrivate { int writing_file; int64_t bytes_written; struct ArrowIpcFooter footer; + + struct ArrowBuffer dictionary_cache; +}; + +struct ArrowIpcWriterDictionaryCacheEntry { + int64_t dictionary_id; + struct ArrowBuffer metadata; + struct ArrowBuffer body; }; +static void ArrowIpcWriterResetDictionaryCache( + struct ArrowIpcWriterPrivate* private) { + int64_t n_cached_dictionaries = + private->dictionary_cache.size_bytes / + (int64_t)sizeof(struct ArrowIpcWriterDictionaryCacheEntry); + struct ArrowIpcWriterDictionaryCacheEntry* cached_dictionaries = + (struct ArrowIpcWriterDictionaryCacheEntry*)private->dictionary_cache.data; + for (int64_t i = 0; i < n_cached_dictionaries; i++) { + ArrowBufferReset(&cached_dictionaries[i].metadata); + ArrowBufferReset(&cached_dictionaries[i].body); + } + ArrowBufferReset(&private->dictionary_cache); +} + ArrowErrorCode ArrowIpcWriterInit(struct ArrowIpcWriter* writer, struct ArrowIpcOutputStream* output_stream) { NANOARROW_DCHECK(writer != NULL && output_stream != NULL); @@ -202,6 +224,7 @@ ArrowErrorCode ArrowIpcWriterInit(struct ArrowIpcWriter* writer, private->writing_file = 0; private->bytes_written = 0; ArrowIpcFooterInit(&private->footer); + ArrowBufferInit(&private->dictionary_cache); writer->private_data = private; return NANOARROW_OK; @@ -221,6 +244,8 @@ void ArrowIpcWriterReset(struct ArrowIpcWriter* writer) { ArrowIpcFooterReset(&private->footer); + ArrowIpcWriterResetDictionaryCache(private); + ArrowFree(private); } memset(writer, 0, sizeof(struct ArrowIpcWriter)); @@ -265,6 +290,7 @@ ArrowErrorCode ArrowIpcWriterWriteSchema(struct ArrowIpcWriter* writer, struct ArrowIpcWriterPrivate* private = (struct ArrowIpcWriterPrivate*)writer->private_data; + ArrowIpcWriterResetDictionaryCache(private); NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSchema(&private->encoder, in, error)); @@ -328,35 +354,11 @@ ArrowErrorCode ArrowIpcWriterWriteArrayView(struct ArrowIpcWriter* writer, return NANOARROW_OK; } -ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( - struct ArrowIpcWriter* writer, int64_t dictionary_id, char is_delta, - const struct ArrowArrayView* values_view, struct ArrowError* error) { - NANOARROW_DCHECK(writer != NULL && writer->private_data != NULL && values_view != NULL); +static ArrowErrorCode ArrowIpcWriterWriteEncodedDictionaryBatch( + struct ArrowIpcWriter* writer, struct ArrowError* error) { struct ArrowIpcWriterPrivate* private = (struct ArrowIpcWriterPrivate*)writer->private_data; - // This check is intentionally minimal: we're allowed to write one dictionary - // batch per ID in a file but we would need to add bookkeeping to keep track - // of written IDs (and usefully a fingerprint or reference to the dictionary - // so we can check if we need to emit it again). - if (private->writing_file && - (is_delta || private->footer.dictionary_blocks.size_bytes != 0)) { - ArrowErrorSet(error, - "IPC file writing supports exactly one non-delta dictionary batch"); - return ENOTSUP; - } - - NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); - NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); - - NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSimpleDictionaryBatch( - &private->encoder, dictionary_id, is_delta, values_view, &private->body_buffer, - error)); - NANOARROW_RETURN_NOT_OK_WITH_ERROR( - ArrowIpcEncoderFinalizeBuffer(&private->encoder, /*encapsulate=*/1, - &private->buffer), - error); - if (private->writing_file) { _NANOARROW_CHECK_RANGE(private->buffer.size_bytes, 0, INT32_MAX); struct ArrowIpcFileBlock block = { @@ -378,21 +380,133 @@ ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( return NANOARROW_OK; } +ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( + struct ArrowIpcWriter* writer, int64_t dictionary_id, char is_delta, + const struct ArrowArrayView* values_view, struct ArrowError* error) { + NANOARROW_DCHECK(writer != NULL && writer->private_data != NULL && values_view != NULL); + struct ArrowIpcWriterPrivate* private = + (struct ArrowIpcWriterPrivate*)writer->private_data; + + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); + + NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSimpleDictionaryBatch( + &private->encoder, dictionary_id, is_delta, values_view, &private->body_buffer, + error)); + NANOARROW_RETURN_NOT_OK_WITH_ERROR( + ArrowIpcEncoderFinalizeBuffer(&private->encoder, /*encapsulate=*/1, + &private->buffer), + error); + + return ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); +} + +static int ArrowIpcWriterBufferEquals(const struct ArrowBuffer* lhs, + const struct ArrowBuffer* rhs) { + return lhs->size_bytes == rhs->size_bytes && + (lhs->size_bytes == 0 || memcmp(lhs->data, rhs->data, lhs->size_bytes) == 0); +} + +static struct ArrowIpcWriterDictionaryCacheEntry* ArrowIpcWriterFindDictionaryCacheEntry( + struct ArrowIpcWriterPrivate* private, int64_t dictionary_id) { + int64_t n_cached_dictionaries = + private->dictionary_cache.size_bytes / + (int64_t)sizeof(struct ArrowIpcWriterDictionaryCacheEntry); + struct ArrowIpcWriterDictionaryCacheEntry* cached_dictionaries = + (struct ArrowIpcWriterDictionaryCacheEntry*)private->dictionary_cache.data; + for (int64_t i = 0; i < n_cached_dictionaries; i++) { + if (cached_dictionaries[i].dictionary_id == dictionary_id) { + return &cached_dictionaries[i]; + } + } + + return NULL; +} + +static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( + struct ArrowIpcWriter* writer, int64_t dictionary_id, + const struct ArrowArrayView* values_view, struct ArrowError* error) { + struct ArrowIpcWriterPrivate* private = + (struct ArrowIpcWriterPrivate*)writer->private_data; + + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); + + NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSimpleDictionaryBatch( + &private->encoder, dictionary_id, /*is_delta=*/0, values_view, + &private->body_buffer, error)); + NANOARROW_RETURN_NOT_OK_WITH_ERROR( + ArrowIpcEncoderFinalizeBuffer(&private->encoder, /*encapsulate=*/1, + &private->buffer), + error); + + struct ArrowIpcWriterDictionaryCacheEntry* cached = + ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); + if (cached != NULL && ArrowIpcWriterBufferEquals(&cached->metadata, &private->buffer) && + ArrowIpcWriterBufferEquals(&cached->body, &private->body_buffer)) { + return NANOARROW_OK; + } + + struct ArrowBuffer metadata_copy; + struct ArrowBuffer body_copy; + ArrowBufferInit(&metadata_copy); + ArrowBufferInit(&body_copy); + ArrowErrorCode result = + ArrowBufferAppend(&metadata_copy, private->buffer.data, private->buffer.size_bytes); + if (result == NANOARROW_OK) { + result = ArrowBufferAppend(&body_copy, private->body_buffer.data, + private->body_buffer.size_bytes); + } + + if (result != NANOARROW_OK) { + ArrowBufferReset(&metadata_copy); + ArrowBufferReset(&body_copy); + return result; + } + + if (cached == NULL) { + struct ArrowIpcWriterDictionaryCacheEntry new_entry = { + .dictionary_id = dictionary_id, + }; + ArrowBufferInit(&new_entry.metadata); + ArrowBufferInit(&new_entry.body); + result = ArrowBufferAppend(&private->dictionary_cache, &new_entry, sizeof(new_entry)); + if (result != NANOARROW_OK) { + ArrowBufferReset(&metadata_copy); + ArrowBufferReset(&body_copy); + return result; + } + cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); + NANOARROW_DCHECK(cached != NULL); + } + + result = ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); + if (result != NANOARROW_OK) { + ArrowBufferReset(&metadata_copy); + ArrowBufferReset(&body_copy); + return result; + } + + ArrowBufferReset(&cached->metadata); + ArrowBufferReset(&cached->body); + ArrowBufferMove(&metadata_copy, &cached->metadata); + ArrowBufferMove(&body_copy, &cached->body); + return NANOARROW_OK; +} + // Walk the array in the same depth-first order the schema encoder uses to assign // dictionary ids (see ArrowIpcDictionaryEncodingsAppendSchema): a dictionary-encoded // node claims the next id before descending into its children and then its values. -// Emitting a full (non-delta) DictionaryBatch for each dictionary before every -// RecordBatch keeps each batch's indices valid against the dictionary that precedes -// it, which is required because each array in the stream carries its own dictionary. -// In the future we can reduce the number of dictionaries emitted by checking for -// identical dictionary arrays. +// Emit a full (non-delta) DictionaryBatch before the first RecordBatch and whenever +// the serialized dictionary changes. Each array in the input stream carries its own +// dictionary, but identical dictionaries do not need to be repeated in the IPC stream. static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( struct ArrowIpcWriter* writer, const struct ArrowArrayView* array_view, int64_t* next_id, struct ArrowError* error) { if (array_view->dictionary != NULL) { int64_t dictionary_id = (*next_id)++; - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionaryBatch( - writer, dictionary_id, /*is_delta=*/0, array_view->dictionary, error)); + NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionaryBatchIfChanged( + writer, dictionary_id, array_view->dictionary, error)); } for (int64_t i = 0; i < array_view->n_children; i++) { diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 3b08d0c89..2636f1544 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -303,7 +303,9 @@ TEST(NanoarrowIpcWriter, WriteDictionaryBatch) { // Build a struct array with a single dictionary-encoded (int32 -> utf8) child. static void MakeDictionaryStructArray(struct ArrowArray* array, - struct ArrowSchema* schema) { + struct ArrowSchema* schema, + const char* value0 = "foo", + const char* value1 = "bar") { ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), @@ -319,8 +321,8 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, struct ArrowArray* values = indices->dictionary; ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView("foo")), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView("bar")), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value0)), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value1)), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(indices, 0), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(indices, 1), NANOARROW_OK); @@ -330,6 +332,111 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, ASSERT_EQ(ArrowArrayFinishBuildingDefault(array, nullptr), NANOARROW_OK); } +static std::vector DecodeMessageTypes(const struct ArrowBuffer* buffer) { + std::vector message_types; + struct ArrowBufferView remaining; + remaining.data.as_uint8 = buffer->data; + remaining.size_bytes = buffer->size_bytes; + struct ArrowIpcDecoder decoder; + struct ArrowError error; + ArrowIpcDecoderInit(&decoder); + + while (remaining.size_bytes > 0) { + int result = ArrowIpcDecoderVerifyHeader(&decoder, remaining, &error); + if (result == ENODATA) { + break; + } + + EXPECT_EQ(result, NANOARROW_OK) << error.message; + if (result != NANOARROW_OK) { + break; + } + + message_types.push_back(decoder.message_type); + int64_t message_size = ((decoder.header_size_bytes + 7) / 8) * 8 + + ((decoder.body_size_bytes + 7) / 8) * 8; + EXPECT_LE(message_size, remaining.size_bytes); + if (message_size > remaining.size_bytes) { + break; + } + + remaining.data.as_uint8 += message_size; + remaining.size_bytes -= message_size; + } + + ArrowIpcDecoderReset(&decoder); + return message_types; +} + +TEST(NanoarrowIpcWriter, DoesNotRepeatUnchangedDictionary) { + struct ArrowError error; + + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get()); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + std::vector message_types = DecodeMessageTypes(output.get()); + EXPECT_EQ(message_types, + (std::vector{NANOARROW_IPC_MESSAGE_TYPE_SCHEMA, + NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH})); +} + +TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { + struct ArrowError error; + + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get(), "foo", "baz"); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + std::vector message_types = DecodeMessageTypes(output.get()); + EXPECT_EQ(message_types, + (std::vector{NANOARROW_IPC_MESSAGE_TYPE_SCHEMA, + NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH})); +} + // Write a dictionary-encoded stream through the high-level WriteArrayStream path // and read it back through the IPC reader, confirming the DictionaryBatch is // emitted automatically and the decoded values match. From 92067f043f7c1b2973fd8e408f02b1e5b68e5679 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 19:28:09 -0400 Subject: [PATCH 03/16] fix(ipc): advertise dictionary replacements --- src/nanoarrow/ipc/encoder.c | 21 +++++++-- src/nanoarrow/ipc/encoder_test.cc | 36 +++++++++++++++ src/nanoarrow/ipc/reader.c | 8 ---- src/nanoarrow/ipc/writer.c | 2 + src/nanoarrow/ipc/writer_test.cc | 77 +++++++++++++++++++++++++++++++ src/nanoarrow/nanoarrow_ipc.h | 11 +++++ 6 files changed, 144 insertions(+), 11 deletions(-) diff --git a/src/nanoarrow/ipc/encoder.c b/src/nanoarrow/ipc/encoder.c index ffc9d6f8e..71e9d795b 100644 --- a/src/nanoarrow/ipc/encoder.c +++ b/src/nanoarrow/ipc/encoder.c @@ -46,6 +46,7 @@ struct ArrowIpcEncoderPrivate { struct ArrowBuffer buffers; struct ArrowBuffer nodes; int encoding_footer; + int dictionary_replacement; struct ArrowIpcDictionaryEncodings dictionary_encodings; // Metadata to attach to the next encoded Message (in nanoarrow's packed // representation), or an empty buffer if the next Message has no metadata. @@ -76,6 +77,7 @@ ArrowErrorCode ArrowIpcEncoderInit(struct ArrowIpcEncoder* encoder) { return ESPIPE; } private->encoding_footer = 0; + private->dictionary_replacement = 0; ArrowBufferInit(&private->buffers); ArrowBufferInit(&private->nodes); ArrowIpcDictionaryEncodingsInit(&private->dictionary_encodings); @@ -216,6 +218,14 @@ ArrowErrorCode ArrowIpcEncoderSetCompression( return ArrowIpcEncoderSetCompressor(encoder, &compressor); } +void ArrowIpcEncoderSetDictionaryReplacement(struct ArrowIpcEncoder* encoder, + char enabled) { + NANOARROW_DCHECK(encoder != NULL && encoder->private_data != NULL); + struct ArrowIpcEncoderPrivate* private = + (struct ArrowIpcEncoderPrivate*)encoder->private_data; + private->dictionary_replacement = enabled != 0; +} + static ArrowErrorCode ArrowIpcEncoderWriteContinuationAndSize(struct ArrowBuffer* out, size_t size) { _NANOARROW_CHECK_UPPER_LIMIT(size, INT32_MAX); @@ -658,7 +668,7 @@ static ArrowErrorCode ArrowIpcEncodeField( static ArrowErrorCode ArrowIpcEncodeSchema( flatcc_builder_t* builder, const struct ArrowSchema* schema, const struct ArrowIpcDictionaryEncodings* dictionary_encodings, int compressed_body, - struct ArrowError* error) { + int dictionary_replacement, struct ArrowError* error) { NANOARROW_DCHECK(schema->release != NULL); if (strcmp(schema->format, "+s") != 0) { @@ -695,6 +705,10 @@ static ArrowErrorCode ArrowIpcEncodeSchema( ns(Feature_enum_t) feature = ns(Feature_COMPRESSED_BODY); FLATCC_RETURN_IF_NULL(ns(Feature_vec_push(builder, &feature)), error); } + if (dictionary_replacement && dictionary_encodings->encodings.size_bytes > 0) { + ns(Feature_enum_t) feature = ns(Feature_DICTIONARY_REPLACEMENT); + FLATCC_RETURN_IF_NULL(ns(Feature_vec_push(builder, &feature)), error); + } FLATCC_RETURN_UNLESS_0(Schema_features_end(builder), error); return NANOARROW_OK; @@ -727,7 +741,8 @@ ArrowErrorCode ArrowIpcEncoderEncodeSchema(struct ArrowIpcEncoder* encoder, NANOARROW_RETURN_NOT_OK(ArrowIpcEncodeSchema( builder, schema, &private->dictionary_encodings, - ArrowIpcEncoderCodec(private) != NANOARROW_IPC_COMPRESSION_TYPE_NONE, error)); + ArrowIpcEncoderCodec(private) != NANOARROW_IPC_COMPRESSION_TYPE_NONE, + private->dictionary_replacement, error)); FLATCC_RETURN_UNLESS_0(Message_header_Schema_end(builder), error); @@ -1183,7 +1198,7 @@ ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder, builder, &footer->schema, &footer->dictionaries, private->has_compressed_body || ArrowIpcEncoderCodec(private) != NANOARROW_IPC_COMPRESSION_TYPE_NONE, - error)); + /*dictionary_replacement=*/0, error)); FLATCC_RETURN_UNLESS_0(Footer_schema_end(builder), error); const struct ArrowIpcFileBlock* blocks = diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index 6457e09ea..f8f838cbb 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -326,6 +326,42 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderSchemaMessageMetadata) { (KeyValues{{"schema_key", "schema_value"}})); } +TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryReplacementFeature) { + nanoarrow::UniqueSchema schema; + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "dict_col"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0]->dictionary, + NANOARROW_TYPE_STRING), + NANOARROW_OK); + + nanoarrow::ipc::UniqueEncoder encoder; + ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); + ArrowIpcEncoderSetDictionaryReplacement(encoder.get(), /*enabled=*/1); + + struct ArrowError error; + nanoarrow::UniqueBuffer message; + ASSERT_EQ(ArrowIpcEncoderEncodeSchema(encoder.get(), schema.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/true, message.get()), + NANOARROW_OK); + + nanoarrow::ipc::UniqueDecoder decoder; + ASSERT_EQ(ArrowIpcDecoderInit(decoder.get()), NANOARROW_OK); + struct ArrowBufferView view; + view.data.data = message->data; + view.size_bytes = message->size_bytes; + ASSERT_EQ(ArrowIpcDecoderDecodeHeader(decoder.get(), view, &error), NANOARROW_OK) + << error.message; + EXPECT_EQ(decoder->feature_flags, NANOARROW_IPC_FEATURE_DICTIONARY_REPLACEMENT); +} + TEST(NanoarrowIpcTest, NanoarrowIpcEncoderMessageMetadataEmpty) { nanoarrow::ipc::UniqueEncoder encoder; ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); diff --git a/src/nanoarrow/ipc/reader.c b/src/nanoarrow/ipc/reader.c index c699ca331..1cef35d5f 100644 --- a/src/nanoarrow/ipc/reader.c +++ b/src/nanoarrow/ipc/reader.c @@ -389,14 +389,6 @@ static int ArrowIpcArrayStreamReaderReadSchemaIfNeeded( return EINVAL; } - // ...or if it uses features we don't support - if (private_data->decoder.feature_flags & - NANOARROW_IPC_FEATURE_DICTIONARY_REPLACEMENT) { - ArrowErrorSet(&private_data->error, - "This stream uses unsupported feature DICTIONARY_REPLACEMENT"); - return EINVAL; - } - // Notify the decoder of buffer endianness NANOARROW_RETURN_NOT_OK_WITH_ERROR( ArrowIpcDecoderSetEndianness(&private_data->decoder, diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 6d9c1c31b..1d1c577bd 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -291,6 +291,8 @@ ArrowErrorCode ArrowIpcWriterWriteSchema(struct ArrowIpcWriter* writer, (struct ArrowIpcWriterPrivate*)writer->private_data; ArrowIpcWriterResetDictionaryCache(private); + ArrowIpcEncoderSetDictionaryReplacement(&private->encoder, + /*enabled=*/!private->writing_file); NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSchema(&private->encoder, in, error)); diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 2636f1544..5446c964d 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -19,6 +19,12 @@ #include +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) +#include +#include +#include +#endif + #include "nanoarrow/nanoarrow_ipc.hpp" TEST(NanoarrowIpcWriter, OutputStreamBuffer) { @@ -435,6 +441,77 @@ TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH, NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH, NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH})); + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_input = std::make_shared( + arrow::Buffer::Wrap(output->data, output->size_bytes)); + auto maybe_arrow_reader = arrow::ipc::RecordBatchStreamReader::Open(arrow_input); + ASSERT_TRUE(maybe_arrow_reader.ok()) << maybe_arrow_reader.status(); + auto arrow_reader = maybe_arrow_reader.ValueUnsafe(); + + std::shared_ptr arrow_batch1; + std::shared_ptr arrow_batch2; + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch1).ok()); + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch2).ok()); + auto arrow_dictionary1 = + std::static_pointer_cast(arrow_batch1->column(0)); + auto arrow_dictionary2 = + std::static_pointer_cast(arrow_batch2->column(0)); + auto arrow_values1 = + std::static_pointer_cast(arrow_dictionary1->dictionary()); + auto arrow_values2 = + std::static_pointer_cast(arrow_dictionary2->dictionary()); + EXPECT_EQ(arrow_values1->GetString(1), "bar"); + EXPECT_EQ(arrow_values2->GetString(1), "baz"); +#endif + + struct ArrowIpcInputStream input; + ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); + nanoarrow::UniqueArrayStream reader; + ASSERT_EQ(ArrowIpcArrayStreamReaderInit(reader.get(), &input, nullptr), NANOARROW_OK); + + nanoarrow::UniqueSchema roundtrip_schema; + ASSERT_EQ(ArrowArrayStreamGetSchema(reader.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) + << error.message; + + nanoarrow::UniqueArray roundtrip_array1; + nanoarrow::UniqueArray roundtrip_array2; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array1.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array2.get(), &error), + NANOARROW_OK) + << error.message; + + nanoarrow::UniqueArrayView roundtrip_view1; + nanoarrow::UniqueArrayView roundtrip_view2; + ASSERT_EQ(ArrowArrayViewInitFromSchema(roundtrip_view1.get(), roundtrip_schema.get(), + &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayViewInitFromSchema(roundtrip_view2.get(), roundtrip_schema.get(), + &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayViewSetArray(roundtrip_view1.get(), roundtrip_array1.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayViewSetArray(roundtrip_view2.get(), roundtrip_array2.get(), &error), + NANOARROW_OK) + << error.message; + + struct ArrowStringView first_dictionary_value = + ArrowArrayViewGetStringUnsafe(roundtrip_view1->children[0]->dictionary, 1); + struct ArrowStringView replacement_dictionary_value = + ArrowArrayViewGetStringUnsafe(roundtrip_view2->children[0]->dictionary, 1); + EXPECT_EQ(std::string(first_dictionary_value.data, + first_dictionary_value.size_bytes), + "bar"); + EXPECT_EQ(std::string(replacement_dictionary_value.data, + replacement_dictionary_value.size_bytes), + "baz"); + } // Write a dictionary-encoded stream through the high-level WriteArrayStream path diff --git a/src/nanoarrow/nanoarrow_ipc.h b/src/nanoarrow/nanoarrow_ipc.h index 87ee87cc6..58b3cc1ac 100644 --- a/src/nanoarrow/nanoarrow_ipc.h +++ b/src/nanoarrow/nanoarrow_ipc.h @@ -108,6 +108,8 @@ NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowIpcEncoderSetCompression) #define ArrowIpcEncoderSetCompressor \ NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowIpcEncoderSetCompressor) +#define ArrowIpcEncoderSetDictionaryReplacement \ + NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowIpcEncoderSetDictionaryReplacement) #define ArrowIpcEncoderEncodeSchema \ NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowIpcEncoderEncodeSchema) #define ArrowIpcEncoderEncodeSimpleRecordBatch \ @@ -1035,6 +1037,15 @@ NANOARROW_DLL ArrowErrorCode ArrowIpcEncoderSetCompression( NANOARROW_DLL ArrowErrorCode ArrowIpcEncoderSetCompressor( struct ArrowIpcEncoder* encoder, struct ArrowIpcCompressor* compressor); +/// \brief Declare dictionary replacement support in subsequently encoded schemas +/// +/// Enable this before encoding the schema of a stream that may contain more than one +/// non-delta DictionaryBatch with the same dictionary ID. The +/// DICTIONARY_REPLACEMENT feature is only written if the schema contains at least one +/// dictionary-encoded field. This option is disabled by default. +NANOARROW_DLL void ArrowIpcEncoderSetDictionaryReplacement( + struct ArrowIpcEncoder* encoder, char enabled); + /// \brief Encode an ArrowSchema /// /// Returns ENOMEM if allocation fails, NANOARROW_OK otherwise. From 62c94a0f4bc94b3ad417c4b72b676da37e427604 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 22:50:51 -0400 Subject: [PATCH 04/16] fix(ipc): encode nested dictionary value fields --- src/nanoarrow/ipc/encoder.c | 17 ++++------- src/nanoarrow/ipc/encoder_test.cc | 50 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/nanoarrow/ipc/encoder.c b/src/nanoarrow/ipc/encoder.c index 71e9d795b..1b0c8dcf9 100644 --- a/src/nanoarrow/ipc/encoder.c +++ b/src/nanoarrow/ipc/encoder.c @@ -565,6 +565,7 @@ static ArrowErrorCode ArrowIpcEncodeField( struct ArrowSchemaView schema_view; NANOARROW_RETURN_NOT_OK(ArrowSchemaViewInit(&schema_view, schema, error)); + const struct ArrowSchema* value_schema = schema; if (schema_view.type == NANOARROW_TYPE_DICTIONARY) { const struct ArrowIpcDictionaryEncoding* encoding = @@ -633,24 +634,16 @@ static ArrowErrorCode ArrowIpcEncodeField( // Add the dictionary encoding to the field FLATCC_RETURN_UNLESS_0(Field_dictionary_add(builder, dict_encoding_ref), error); - // Support dictionary values with children by encoding children from - // schema->dictionary (and add a roundtrip test for a nested value type). - // Using schema below would encode the index type's children instead and - // produce a Field whose type and children do not agree. - if (schema->dictionary->n_children != 0) { - ArrowErrorSet(error, "IPC encoding of dictionary values with children unsupported"); - return ENOTSUP; - } - - NANOARROW_RETURN_NOT_OK(ArrowSchemaViewInit(&schema_view, schema->dictionary, error)); + value_schema = schema->dictionary; + NANOARROW_RETURN_NOT_OK(ArrowSchemaViewInit(&schema_view, value_schema, error)); } NANOARROW_RETURN_NOT_OK(ArrowIpcEncodeFieldType(builder, &schema_view, error)); - if (schema->n_children != 0) { + if (value_schema->n_children != 0) { FLATCC_RETURN_UNLESS_0(Field_children_start(builder), error); NANOARROW_RETURN_NOT_OK( - ArrowIpcEncodeFields(builder, schema, &ns(Field_children_push_start), + ArrowIpcEncodeFields(builder, value_schema, &ns(Field_children_push_start), &ns(Field_children_push_end), dictionary_encodings, error)); FLATCC_RETURN_UNLESS_0(Field_children_end(builder), error); } diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index f8f838cbb..3fdee9ff3 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -362,6 +362,56 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryReplacementFeature) { EXPECT_EQ(decoder->feature_flags, NANOARROW_IPC_FEATURE_DICTIONARY_REPLACEMENT); } +TEST(NanoarrowIpcTest, NanoarrowIpcEncoderNestedDictionaryValueSchema) { + nanoarrow::UniqueSchema schema; + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); + + struct ArrowSchema* dictionary_field = schema->children[0]; + ASSERT_EQ(ArrowSchemaInitFromType(dictionary_field, NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(dictionary_field, "dict_col"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(dictionary_field), NANOARROW_OK); + + struct ArrowSchema* value_schema = dictionary_field->dictionary; + ASSERT_EQ(ArrowSchemaInitFromType(value_schema, NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(value_schema, 1), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(value_schema->children[0], NANOARROW_TYPE_STRING), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(value_schema->children[0], "value"), NANOARROW_OK); + + nanoarrow::ipc::UniqueEncoder encoder; + ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); + struct ArrowError error; + nanoarrow::UniqueBuffer message; + ASSERT_EQ(ArrowIpcEncoderEncodeSchema(encoder.get(), schema.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/true, message.get()), + NANOARROW_OK); + + nanoarrow::ipc::UniqueDecoder decoder; + ASSERT_EQ(ArrowIpcDecoderInit(decoder.get()), NANOARROW_OK); + struct ArrowBufferView view; + view.data.data = message->data; + view.size_bytes = message->size_bytes; + ASSERT_EQ(ArrowIpcDecoderDecodeHeader(decoder.get(), view, &error), NANOARROW_OK) + << error.message; + + nanoarrow::UniqueSchema roundtrip; + ASSERT_EQ(ArrowIpcDecoderDecodeSchema(decoder.get(), roundtrip.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_NE(roundtrip->children[0]->dictionary, nullptr); + EXPECT_STREQ(roundtrip->children[0]->dictionary->format, "+s"); + ASSERT_EQ(roundtrip->children[0]->dictionary->n_children, 1); + EXPECT_STREQ(roundtrip->children[0]->dictionary->children[0]->format, "u"); + EXPECT_STREQ(roundtrip->children[0]->dictionary->children[0]->name, "value"); +} + TEST(NanoarrowIpcTest, NanoarrowIpcEncoderMessageMetadataEmpty) { nanoarrow::ipc::UniqueEncoder encoder; ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); From 6a1fbd5bdfe3843dafc07a9ee1d8e49ed0b58385 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 22:52:07 -0400 Subject: [PATCH 05/16] fix(ipc): write nested dictionaries dependency first --- src/nanoarrow/ipc/writer.c | 63 ++++++++++++---- src/nanoarrow/ipc/writer_test.cc | 119 +++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 12 deletions(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 1d1c577bd..c16664eac 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -191,6 +191,11 @@ struct ArrowIpcWriterDictionaryCacheEntry { struct ArrowBuffer body; }; +struct ArrowIpcWriterDictionaryView { + int64_t dictionary_id; + const struct ArrowArrayView* values_view; +}; + static void ArrowIpcWriterResetDictionaryCache( struct ArrowIpcWriterPrivate* private) { int64_t n_cached_dictionaries = @@ -502,28 +507,63 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( // Emit a full (non-delta) DictionaryBatch before the first RecordBatch and whenever // the serialized dictionary changes. Each array in the input stream carries its own // dictionary, but identical dictionaries do not need to be repeated in the IPC stream. -static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( - struct ArrowIpcWriter* writer, const struct ArrowArrayView* array_view, - int64_t* next_id, struct ArrowError* error) { +static ArrowErrorCode ArrowIpcWriterCollectDictionariesForArrayView( + const struct ArrowArrayView* array_view, struct ArrowBuffer* dictionaries, + int64_t* next_id) { if (array_view->dictionary != NULL) { - int64_t dictionary_id = (*next_id)++; - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionaryBatchIfChanged( - writer, dictionary_id, array_view->dictionary, error)); + struct ArrowIpcWriterDictionaryView dictionary = { + .dictionary_id = (*next_id)++, + .values_view = array_view->dictionary, + }; + NANOARROW_RETURN_NOT_OK( + ArrowBufferAppend(dictionaries, &dictionary, sizeof(dictionary))); } for (int64_t i = 0; i < array_view->n_children; i++) { - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionariesForArrayView( - writer, array_view->children[i], next_id, error)); + NANOARROW_RETURN_NOT_OK(ArrowIpcWriterCollectDictionariesForArrayView( + array_view->children[i], dictionaries, next_id)); } if (array_view->dictionary != NULL) { - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionariesForArrayView( - writer, array_view->dictionary, next_id, error)); + NANOARROW_RETURN_NOT_OK(ArrowIpcWriterCollectDictionariesForArrayView( + array_view->dictionary, dictionaries, next_id)); } return NANOARROW_OK; } +static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( + struct ArrowIpcWriter* writer, const struct ArrowArrayView* array_view, + struct ArrowError* error) { + struct ArrowBuffer dictionaries; + ArrowBufferInit(&dictionaries); + int64_t next_id = 0; + ArrowErrorCode result = ArrowIpcWriterCollectDictionariesForArrayView( + array_view, &dictionaries, &next_id); + + if (result == NANOARROW_OK) { + const struct ArrowIpcWriterDictionaryView* dictionary_views = + (const struct ArrowIpcWriterDictionaryView*)dictionaries.data; + int64_t n_dictionaries = + dictionaries.size_bytes / sizeof(struct ArrowIpcWriterDictionaryView); + + // Dictionary IDs are assigned in schema traversal order. Write in reverse + // traversal order so dictionaries nested in another dictionary's values are + // available before decoding their parent. + for (int64_t i = n_dictionaries - 1; i >= 0; i--) { + result = ArrowIpcWriterWriteDictionaryBatchIfChanged( + writer, dictionary_views[i].dictionary_id, + dictionary_views[i].values_view, error); + if (result != NANOARROW_OK) { + break; + } + } + } + + ArrowBufferReset(&dictionaries); + return result; +} + static ArrowErrorCode ArrowIpcWriterWriteArrayStreamImpl( struct ArrowIpcWriter* writer, struct ArrowArrayStream* in, struct ArrowSchema* schema, struct ArrowArray* array, @@ -540,9 +580,8 @@ static ArrowErrorCode ArrowIpcWriterWriteArrayStreamImpl( NANOARROW_RETURN_NOT_OK(ArrowArrayViewSetArray(array_view, array, error)); - int64_t next_dictionary_id = 0; NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionariesForArrayView( - writer, array_view, &next_dictionary_id, error)); + writer, array_view, error)); NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteArrayView(writer, array_view, error)); ArrowArrayRelease(array); diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 5446c964d..284d75984 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -374,6 +374,125 @@ static std::vector DecodeMessageTypes(const struct ArrowBuffer* buffer) return message_types; } +static std::vector DecodeDictionaryIds(const struct ArrowBuffer* buffer) { + std::vector dictionary_ids; + struct ArrowBufferView remaining; + remaining.data.as_uint8 = buffer->data; + remaining.size_bytes = buffer->size_bytes; + struct ArrowIpcDecoder decoder; + struct ArrowError error; + ArrowIpcDecoderInit(&decoder); + + while (remaining.size_bytes > 0) { + int result = ArrowIpcDecoderVerifyHeader(&decoder, remaining, &error); + if (result == ENODATA) { + break; + } + + EXPECT_EQ(result, NANOARROW_OK) << error.message; + if (result != NANOARROW_OK) { + break; + } + + if (decoder.message_type == NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH) { + result = ArrowIpcDecoderDecodeHeader(&decoder, remaining, &error); + EXPECT_EQ(result, NANOARROW_OK) << error.message; + if (result != NANOARROW_OK) { + break; + } + dictionary_ids.push_back(decoder.dictionary->id); + } + + int64_t message_size = ((decoder.header_size_bytes + 7) / 8) * 8 + + ((decoder.body_size_bytes + 7) / 8) * 8; + remaining.data.as_uint8 += message_size; + remaining.size_bytes -= message_size; + } + + ArrowIpcDecoderReset(&decoder); + return dictionary_ids; +} + +static void MakeNestedDictionaryStructArray(struct ArrowArray* array, + struct ArrowSchema* schema) { + ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); + + struct ArrowSchema* outer_field = schema->children[0]; + ASSERT_EQ(ArrowSchemaInitFromType(outer_field, NANOARROW_TYPE_INT32), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(outer_field, "outer"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(outer_field), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(outer_field->dictionary, NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(outer_field->dictionary, 1), NANOARROW_OK); + + struct ArrowSchema* inner_field = outer_field->dictionary->children[0]; + ASSERT_EQ(ArrowSchemaInitFromType(inner_field, NANOARROW_TYPE_INT32), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(inner_field, "inner"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(inner_field), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(inner_field->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); + + ASSERT_EQ(ArrowArrayInitFromSchema(array, schema, nullptr), NANOARROW_OK); + struct ArrowArray* outer_indices = array->children[0]; + struct ArrowArray* outer_values = outer_indices->dictionary; + struct ArrowArray* inner_indices = outer_values->children[0]; + struct ArrowArray* inner_values = inner_indices->dictionary; + + ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(inner_values, ArrowCharView("foo")), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(inner_values, ArrowCharView("bar")), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(inner_indices, 0), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(inner_indices, 1), NANOARROW_OK); + outer_values->length = 2; + ASSERT_EQ(ArrowArrayAppendInt(outer_indices, 0), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(outer_indices, 1), NANOARROW_OK); + array->length = 2; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(array, nullptr), NANOARROW_OK); +} + +TEST(NanoarrowIpcWriter, WritesNestedDictionariesDependencyFirst) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array; + MakeNestedDictionaryStructArray(array.get(), schema.get()); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 1), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + EXPECT_EQ(DecodeDictionaryIds(output.get()), (std::vector{1, 0})); + + struct ArrowIpcInputStream input; + ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); + nanoarrow::UniqueArrayStream reader; + ASSERT_EQ(ArrowIpcArrayStreamReaderInit(reader.get(), &input, nullptr), NANOARROW_OK); + nanoarrow::UniqueSchema roundtrip_schema; + ASSERT_EQ(ArrowArrayStreamGetSchema(reader.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) + << error.message; + nanoarrow::UniqueArray roundtrip_array; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array.get(), &error), + NANOARROW_OK) + << error.message; + + struct ArrowArray* outer_dictionary = roundtrip_array->children[0]->dictionary; + ASSERT_NE(outer_dictionary, nullptr); + ASSERT_EQ(outer_dictionary->n_children, 1); + struct ArrowArray* inner_dictionary = outer_dictionary->children[0]->dictionary; + ASSERT_NE(inner_dictionary, nullptr); + EXPECT_EQ(inner_dictionary->length, 2); +} + TEST(NanoarrowIpcWriter, DoesNotRepeatUnchangedDictionary) { struct ArrowError error; From 7d62f80769470cccb4b5226904e08a67338f75d0 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 23:14:00 -0400 Subject: [PATCH 06/16] fix(ipc): invalidate nested dictionary dependents --- src/nanoarrow/ipc/writer.c | 43 +++++++++++++++----- src/nanoarrow/ipc/writer_test.cc | 67 +++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 12 deletions(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index c16664eac..e545b789e 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -191,9 +191,13 @@ struct ArrowIpcWriterDictionaryCacheEntry { struct ArrowBuffer body; }; +#define NANOARROW_IPC_NO_PARENT_DICTIONARY_ID -1 + struct ArrowIpcWriterDictionaryView { int64_t dictionary_id; + int64_t parent_dictionary_id; const struct ArrowArrayView* values_view; + int force_emit; }; static void ArrowIpcWriterResetDictionaryCache( @@ -432,7 +436,8 @@ static struct ArrowIpcWriterDictionaryCacheEntry* ArrowIpcWriterFindDictionaryCa static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( struct ArrowIpcWriter* writer, int64_t dictionary_id, - const struct ArrowArrayView* values_view, struct ArrowError* error) { + const struct ArrowArrayView* values_view, int force_emit, int* emitted, + struct ArrowError* error) { struct ArrowIpcWriterPrivate* private = (struct ArrowIpcWriterPrivate*)writer->private_data; @@ -449,8 +454,10 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( struct ArrowIpcWriterDictionaryCacheEntry* cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); - if (cached != NULL && ArrowIpcWriterBufferEquals(&cached->metadata, &private->buffer) && + if (!force_emit && cached != NULL && + ArrowIpcWriterBufferEquals(&cached->metadata, &private->buffer) && ArrowIpcWriterBufferEquals(&cached->body, &private->body_buffer)) { + *emitted = 0; return NANOARROW_OK; } @@ -498,6 +505,7 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( ArrowBufferReset(&cached->body); ArrowBufferMove(&metadata_copy, &cached->metadata); ArrowBufferMove(&body_copy, &cached->body); + *emitted = 1; return NANOARROW_OK; } @@ -509,11 +517,15 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( // dictionary, but identical dictionaries do not need to be repeated in the IPC stream. static ArrowErrorCode ArrowIpcWriterCollectDictionariesForArrayView( const struct ArrowArrayView* array_view, struct ArrowBuffer* dictionaries, - int64_t* next_id) { + int64_t* next_id, int64_t parent_dictionary_id) { + int64_t dictionary_id = NANOARROW_IPC_NO_PARENT_DICTIONARY_ID; if (array_view->dictionary != NULL) { + dictionary_id = (*next_id)++; struct ArrowIpcWriterDictionaryView dictionary = { - .dictionary_id = (*next_id)++, + .dictionary_id = dictionary_id, + .parent_dictionary_id = parent_dictionary_id, .values_view = array_view->dictionary, + .force_emit = 0, }; NANOARROW_RETURN_NOT_OK( ArrowBufferAppend(dictionaries, &dictionary, sizeof(dictionary))); @@ -521,12 +533,12 @@ static ArrowErrorCode ArrowIpcWriterCollectDictionariesForArrayView( for (int64_t i = 0; i < array_view->n_children; i++) { NANOARROW_RETURN_NOT_OK(ArrowIpcWriterCollectDictionariesForArrayView( - array_view->children[i], dictionaries, next_id)); + array_view->children[i], dictionaries, next_id, parent_dictionary_id)); } if (array_view->dictionary != NULL) { NANOARROW_RETURN_NOT_OK(ArrowIpcWriterCollectDictionariesForArrayView( - array_view->dictionary, dictionaries, next_id)); + array_view->dictionary, dictionaries, next_id, dictionary_id)); } return NANOARROW_OK; @@ -539,11 +551,12 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( ArrowBufferInit(&dictionaries); int64_t next_id = 0; ArrowErrorCode result = ArrowIpcWriterCollectDictionariesForArrayView( - array_view, &dictionaries, &next_id); + array_view, &dictionaries, &next_id, + NANOARROW_IPC_NO_PARENT_DICTIONARY_ID); if (result == NANOARROW_OK) { - const struct ArrowIpcWriterDictionaryView* dictionary_views = - (const struct ArrowIpcWriterDictionaryView*)dictionaries.data; + struct ArrowIpcWriterDictionaryView* dictionary_views = + (struct ArrowIpcWriterDictionaryView*)dictionaries.data; int64_t n_dictionaries = dictionaries.size_bytes / sizeof(struct ArrowIpcWriterDictionaryView); @@ -551,12 +564,22 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( // traversal order so dictionaries nested in another dictionary's values are // available before decoding their parent. for (int64_t i = n_dictionaries - 1; i >= 0; i--) { + int emitted = 0; result = ArrowIpcWriterWriteDictionaryBatchIfChanged( writer, dictionary_views[i].dictionary_id, - dictionary_views[i].values_view, error); + dictionary_views[i].values_view, dictionary_views[i].force_emit, + &emitted, error); if (result != NANOARROW_OK) { break; } + + // A parent DictionaryBatch captures the current values of dictionaries nested + // within it. Re-emit ancestors after a dependency changes even when the parent's + // own serialized buffers are otherwise identical. + if (emitted && dictionary_views[i].parent_dictionary_id != + NANOARROW_IPC_NO_PARENT_DICTIONARY_ID) { + dictionary_views[dictionary_views[i].parent_dictionary_id].force_emit = 1; + } } } diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 284d75984..65159fd3b 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -414,7 +414,8 @@ static std::vector DecodeDictionaryIds(const struct ArrowBuffer* buffer } static void MakeNestedDictionaryStructArray(struct ArrowArray* array, - struct ArrowSchema* schema) { + struct ArrowSchema* schema, + const char* inner_value1 = "bar") { ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); @@ -441,7 +442,8 @@ static void MakeNestedDictionaryStructArray(struct ArrowArray* array, ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendString(inner_values, ArrowCharView("foo")), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(inner_values, ArrowCharView("bar")), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(inner_values, ArrowCharView(inner_value1)), + NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(inner_indices, 0), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(inner_indices, 1), NANOARROW_OK); outer_values->length = 2; @@ -493,6 +495,67 @@ TEST(NanoarrowIpcWriter, WritesNestedDictionariesDependencyFirst) { EXPECT_EQ(inner_dictionary->length, 2); } +TEST(NanoarrowIpcWriter, ReemitsParentWhenNestedDictionaryChanges) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeNestedDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeNestedDictionaryStructArray(array2.get(), unused_schema.get(), "baz"); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + EXPECT_EQ(DecodeDictionaryIds(output.get()), + (std::vector{1, 0, 1, 0})); + + struct ArrowIpcInputStream input; + ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); + nanoarrow::UniqueArrayStream reader; + ASSERT_EQ(ArrowIpcArrayStreamReaderInit(reader.get(), &input, nullptr), NANOARROW_OK); + nanoarrow::UniqueSchema roundtrip_schema; + ASSERT_EQ(ArrowArrayStreamGetSchema(reader.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) + << error.message; + + nanoarrow::UniqueArray roundtrip_array1; + nanoarrow::UniqueArray roundtrip_array2; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array1.get(), &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array2.get(), &error), + NANOARROW_OK) + << error.message; + + const struct ArrowArray* inner_dictionary1 = + roundtrip_array1->children[0]->dictionary->children[0]->dictionary; + const struct ArrowArray* inner_dictionary2 = + roundtrip_array2->children[0]->dictionary->children[0]->dictionary; + nanoarrow::UniqueArrayView inner_view1; + nanoarrow::UniqueArrayView inner_view2; + ArrowArrayViewInitFromType(inner_view1.get(), NANOARROW_TYPE_STRING); + ArrowArrayViewInitFromType(inner_view2.get(), NANOARROW_TYPE_STRING); + ASSERT_EQ(ArrowArrayViewSetArray(inner_view1.get(), inner_dictionary1, &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(inner_view2.get(), inner_dictionary2, &error), + NANOARROW_OK); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(inner_view1.get(), 1), ArrowCharView("bar")); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(inner_view2.get(), 1), ArrowCharView("baz")); +} + TEST(NanoarrowIpcWriter, DoesNotRepeatUnchangedDictionary) { struct ArrowError error; From 50a94da8514e3ae4546c3bf0eeb1a61e722f5473 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 23:15:17 -0400 Subject: [PATCH 07/16] fix(ipc): reject dictionary mutation in files --- src/nanoarrow/ipc/writer.c | 28 ++++ src/nanoarrow/ipc/writer_test.cc | 222 +++++++++++++++++++++++++++++++ src/nanoarrow/nanoarrow_ipc.h | 3 + 3 files changed, 253 insertions(+) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index e545b789e..70df58de0 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -16,6 +16,7 @@ // under the License. #include +#include #include #include @@ -311,8 +312,17 @@ ArrowErrorCode ArrowIpcWriterWriteSchema(struct ArrowIpcWriter* writer, error); if (private->writing_file) { + if (private->footer.schema.release != NULL) { + ArrowSchemaRelease(&private->footer.schema); + } + ArrowIpcDictionaryEncodingsReset(&private->footer.dictionaries); + ArrowIpcDictionaryEncodingsInit(&private->footer.dictionaries); NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowSchemaDeepCopy(in, &private->footer.schema), error); + NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowIpcDictionaryEncodingsAppendSchema( + &private->footer.dictionaries, + &private->footer.schema), + error); } private->bytes_written += private->buffer.size_bytes; @@ -391,6 +401,11 @@ static ArrowErrorCode ArrowIpcWriterWriteEncodedDictionaryBatch( return NANOARROW_OK; } +static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( + struct ArrowIpcWriter* writer, int64_t dictionary_id, + const struct ArrowArrayView* values_view, int force_emit, int* emitted, + struct ArrowError* error); + ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( struct ArrowIpcWriter* writer, int64_t dictionary_id, char is_delta, const struct ArrowArrayView* values_view, struct ArrowError* error) { @@ -398,6 +413,12 @@ ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( struct ArrowIpcWriterPrivate* private = (struct ArrowIpcWriterPrivate*)writer->private_data; + if (private->writing_file && !is_delta) { + int emitted; + return ArrowIpcWriterWriteDictionaryBatchIfChanged( + writer, dictionary_id, values_view, /*force_emit=*/0, &emitted, error); + } + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); @@ -461,6 +482,13 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( return NANOARROW_OK; } + if (private->writing_file && cached != NULL) { + ArrowErrorSet(error, + "Arrow IPC files do not support replacement of dictionary ID %" PRId64, + dictionary_id); + return EINVAL; + } + struct ArrowBuffer metadata_copy; struct ArrowBuffer body_copy; ArrowBufferInit(&metadata_copy); diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 65159fd3b..4a1f578f7 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -307,6 +307,164 @@ TEST(NanoarrowIpcWriter, WriteDictionaryBatch) { "IPC file writing supports exactly one non-delta dictionary batch"); } +TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "dict_col"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0]->dictionary, + NANOARROW_TYPE_STRING), + NANOARROW_OK); + + nanoarrow::UniqueSchema values_schema; + ASSERT_EQ(ArrowSchemaInitFromType(values_schema.get(), NANOARROW_TYPE_STRING), + NANOARROW_OK); + nanoarrow::UniqueArray full_values; + nanoarrow::UniqueArray delta_values; + ASSERT_EQ(ArrowArrayInitFromSchema(full_values.get(), values_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayInitFromSchema(delta_values.get(), values_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(full_values.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(delta_values.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(full_values.get(), ArrowCharView("zero")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(delta_values.get(), ArrowCharView("one")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(delta_values.get(), ArrowCharView("two")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayFinishBuildingDefault(full_values.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowArrayFinishBuildingDefault(delta_values.get(), &error), NANOARROW_OK); + + nanoarrow::UniqueArray batch1; + nanoarrow::UniqueArray batch2; + ASSERT_EQ(ArrowArrayInitFromSchema(batch1.get(), schema.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowArrayInitFromSchema(batch2.get(), schema.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(batch1.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(batch2.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(batch1->children[0]->dictionary, + ArrowCharView("zero")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(batch1->children[0], 0), NANOARROW_OK); + batch1->length = 1; + ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, + ArrowCharView("zero")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, + ArrowCharView("one")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, + ArrowCharView("two")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(batch2->children[0], 2), NANOARROW_OK); + batch2->length = 1; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(batch1.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowArrayFinishBuildingDefault(batch2.get(), &error), NANOARROW_OK); + + nanoarrow::UniqueArrayView full_values_view; + nanoarrow::UniqueArrayView delta_values_view; + nanoarrow::UniqueArrayView batch1_view; + nanoarrow::UniqueArrayView batch2_view; + ASSERT_EQ(ArrowArrayViewInitFromSchema(full_values_view.get(), values_schema.get(), + &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewInitFromSchema(delta_values_view.get(), values_schema.get(), + &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewInitFromSchema(batch1_view.get(), schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewInitFromSchema(batch2_view.get(), schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(full_values_view.get(), full_values.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(delta_values_view.get(), delta_values.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(batch1_view.get(), batch1.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(batch2_view.get(), batch2.get(), &error), + NANOARROW_OK); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteSchema(writer.get(), schema.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( + writer.get(), 0, /*is_delta=*/0, full_values_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(writer.get(), batch1_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( + writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(writer.get(), batch2_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(writer.get(), nullptr, &error), NANOARROW_OK); + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_input = std::make_shared( + arrow::Buffer::Wrap(output->data, output->size_bytes)); + auto maybe_arrow_reader = arrow::ipc::RecordBatchStreamReader::Open(arrow_input); + ASSERT_TRUE(maybe_arrow_reader.ok()) << maybe_arrow_reader.status(); + auto arrow_reader = maybe_arrow_reader.ValueUnsafe(); + std::shared_ptr arrow_batch1; + std::shared_ptr arrow_batch2; + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch1).ok()); + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch2).ok()); + auto arrow_dictionary1 = + std::static_pointer_cast(arrow_batch1->column(0)); + auto arrow_dictionary2 = + std::static_pointer_cast(arrow_batch2->column(0)); + EXPECT_EQ(arrow_dictionary1->dictionary()->length(), 1); + EXPECT_EQ(arrow_dictionary2->dictionary()->length(), 3); +#endif + + // Files permit deltas (applied in footer order), but not replacement batches. + nanoarrow::UniqueBuffer file_output; + nanoarrow::ipc::UniqueOutputStream file_out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(file_out_stream.get(), file_output.get()), + NANOARROW_OK); + nanoarrow::ipc::UniqueWriter file_writer; + ASSERT_EQ(ArrowIpcWriterInit(file_writer.get(), file_out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterStartFile(file_writer.get(), &error), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteSchema(file_writer.get(), schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( + file_writer.get(), 0, /*is_delta=*/0, full_values_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch1_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( + file_writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch2_view.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), nullptr, &error), + NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterFinalizeFile(file_writer.get(), &error), NANOARROW_OK) + << error.message; + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_file_input = std::make_shared( + arrow::Buffer::Wrap(file_output->data, file_output->size_bytes)); + auto maybe_file_reader = arrow::ipc::RecordBatchFileReader::Open(arrow_file_input); + ASSERT_TRUE(maybe_file_reader.ok()) << maybe_file_reader.status(); + auto arrow_file_reader = maybe_file_reader.ValueUnsafe(); + ASSERT_EQ(arrow_file_reader->num_record_batches(), 2); + auto maybe_file_batch2 = arrow_file_reader->ReadRecordBatch(1); + ASSERT_TRUE(maybe_file_batch2.ok()) << maybe_file_batch2.status(); + auto arrow_file_dictionary2 = std::static_pointer_cast( + maybe_file_batch2.ValueUnsafe()->column(0)); + EXPECT_EQ(arrow_file_dictionary2->dictionary()->length(), 3); +#endif +} + // Build a struct array with a single dictionary-encoded (int32 -> utf8) child. static void MakeDictionaryStructArray(struct ArrowArray* array, struct ArrowSchema* schema, @@ -696,6 +854,70 @@ TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { } +TEST(NanoarrowIpcWriter, RejectsChangedDictionaryInFile) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get(), "foo", "baz"); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) + << error.message; + + EXPECT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + EINVAL); + EXPECT_STREQ(error.message, + "Arrow IPC files do not support replacement of dictionary ID 0"); +} + +TEST(NanoarrowIpcWriter, WritesDeltaDictionaryInFile) { + struct ArrowError error; + nanoarrow::UniqueSchema values_schema; + ASSERT_EQ(ArrowSchemaInitFromType(values_schema.get(), NANOARROW_TYPE_STRING), + NANOARROW_OK); + nanoarrow::UniqueArray values; + ASSERT_EQ(ArrowArrayInitFromSchema(values.get(), values_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(values.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(values.get(), ArrowCharView("delta")), NANOARROW_OK); + ASSERT_EQ(ArrowArrayFinishBuildingDefault(values.get(), &error), NANOARROW_OK); + nanoarrow::UniqueArrayView values_view; + ASSERT_EQ(ArrowArrayViewInitFromSchema(values_view.get(), values_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(values_view.get(), values.get(), &error), + NANOARROW_OK); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) + << error.message; + + EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch( + writer.get(), 0, /*is_delta=*/1, values_view.get(), &error), + NANOARROW_OK) + << error.message; + auto* private_data = + static_cast(writer->private_data); + EXPECT_EQ(private_data->footer.dictionary_blocks.size_bytes, + sizeof(struct ArrowIpcFileBlock)); +} + // Write a dictionary-encoded stream through the high-level WriteArrayStream path // and read it back through the IPC reader, confirming the DictionaryBatch is // emitted automatically and the decoded values match. diff --git a/src/nanoarrow/nanoarrow_ipc.h b/src/nanoarrow/nanoarrow_ipc.h index 58b3cc1ac..40fff14f6 100644 --- a/src/nanoarrow/nanoarrow_ipc.h +++ b/src/nanoarrow/nanoarrow_ipc.h @@ -1180,6 +1180,9 @@ NANOARROW_DLL ArrowErrorCode ArrowIpcWriterWriteArrayView(struct ArrowIpcWriter* /// dictionary_id must match the id assigned to the dictionary-encoded field in the /// schema. is_delta selects DictionaryBatch.isDelta. values_view must not itself be /// dictionary-encoded. The writer does not check that a schema was already written. +/// In file mode, changed non-delta dictionaries previously written with the same ID +/// return EINVAL; an identical repeated dictionary is suppressed. Delta dictionaries +/// are written and recorded in footer order. /// /// Errors are propagated from the underlying encoder and output byte stream. NANOARROW_DLL ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( From c3332e1c57ded738a51127f61b772963d00b57e9 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 23:17:00 -0400 Subject: [PATCH 08/16] test(ipc): enable dictionary write integration cases --- src/nanoarrow/ipc/files_test.cc | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/nanoarrow/ipc/files_test.cc b/src/nanoarrow/ipc/files_test.cc index 73fc9724f..9a377a782 100644 --- a/src/nanoarrow/ipc/files_test.cc +++ b/src/nanoarrow/ipc/files_test.cc @@ -214,7 +214,7 @@ class TestFile { } ArrowErrorCode WriteNanoarrowStream(const nanoarrow::UniqueSchema& schema, - const std::vector& arrays, + std::vector& arrays, enum ArrowIpcCompressionType codec, struct ArrowBuffer* buffer, struct ArrowError* error) { @@ -226,19 +226,25 @@ class TestFile { NANOARROW_RETURN_NOT_OK(ArrowIpcWriterSetCompression( writer.get(), codec, NANOARROW_IPC_COMPRESSION_LEVEL_DEFAULT, error)); - nanoarrow::UniqueArrayView array_view; + nanoarrow::UniqueSchema schema_copy; + NANOARROW_RETURN_NOT_OK(ArrowSchemaDeepCopy(schema.get(), schema_copy.get())); + nanoarrow::UniqueArrayStream array_stream; NANOARROW_RETURN_NOT_OK( - ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), error)); - - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteSchema(writer.get(), schema.get(), error)); - for (const auto& array : arrays) { - NANOARROW_RETURN_NOT_OK( - ArrowArrayViewSetArray(array_view.get(), array.get(), error)); - - NANOARROW_RETURN_NOT_OK( - ArrowIpcWriterWriteArrayView(writer.get(), array_view.get(), error)); + ArrowBasicArrayStreamInit(array_stream.get(), schema_copy.get(), arrays.size())); + + for (size_t i = 0; i < arrays.size(); i++) { + // Preserve the decoded array for the subsequent Arrow C++ comparison while + // giving the basic stream an independently releasable shared clone. + nanoarrow::UniqueArray shared; + nanoarrow::UniqueArray clone; + NANOARROW_RETURN_NOT_OK(ArrowArrayMoveShared(arrays[i].get(), shared.get())); + ArrowErrorCode result = ArrowArrayCloneShared(shared.get(), clone.get()); + ArrowArrayMove(shared.get(), arrays[i].get()); + NANOARROW_RETURN_NOT_OK(result); + ArrowBasicArrayStreamSetArray(array_stream.get(), i, clone.get()); } - return ArrowIpcWriterWriteArrayView(writer.get(), nullptr, error); + + return ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), error); } void TestEqualsArrowCpp(const std::string& dir_prefix, @@ -529,9 +535,9 @@ INSTANTIATE_TEST_SUITE_P( TestFile::OK("generated_primitive.stream"), TestFile::OK("generated_recursive_nested.stream"), TestFile::OK("generated_union.stream"), - TestFile::ReadOnly("generated_dictionary_unsigned.stream"), - TestFile::ReadOnly("generated_dictionary.stream"), - TestFile::ReadOnly("generated_nested_dictionary.stream"), + TestFile::OK("generated_dictionary_unsigned.stream"), + TestFile::OK("generated_dictionary.stream"), + TestFile::OK("generated_nested_dictionary.stream"), TestFile::ReadOnly("generated_extension.stream") // Comment to keep last line from wrapping )); @@ -608,10 +614,10 @@ INSTANTIATE_TEST_SUITE_P( TestFile::OK("cpp-21.0.0/generated_primitive_zerolength.stream"), TestFile::OK("cpp-21.0.0/generated_recursive_nested.stream"), TestFile::OK("cpp-21.0.0/generated_union.stream"), - TestFile::ReadOnly("cpp-21.0.0/generated_dictionary.stream"), - TestFile::ReadOnly("cpp-21.0.0/generated_dictionary_unsigned.stream"), + TestFile::OK("cpp-21.0.0/generated_dictionary.stream"), + TestFile::OK("cpp-21.0.0/generated_dictionary_unsigned.stream"), TestFile::ReadOnly("cpp-21.0.0/generated_extension.stream"), - TestFile::ReadOnly("cpp-21.0.0/generated_nested_dictionary.stream"), + TestFile::OK("cpp-21.0.0/generated_nested_dictionary.stream"), TestFile::NotSupported("cpp-21.0.0/generated_list_view.stream"), TestFile::NotSupported("cpp-21.0.0/generated_binary_view.stream"), TestFile::NotSupported("cpp-21.0.0/generated_run_end_encoded.stream") From aab430bb3754993ee41e99370d02d58b65cd1d75 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 23:33:51 -0400 Subject: [PATCH 09/16] perf(ipc): reduce dictionary cache copies --- src/nanoarrow/ipc/writer.c | 43 ++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 70df58de0..146d7ef27 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -478,6 +478,8 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( if (!force_emit && cached != NULL && ArrowIpcWriterBufferEquals(&cached->metadata, &private->buffer) && ArrowIpcWriterBufferEquals(&cached->body, &private->body_buffer)) { + ArrowBufferReset(&private->buffer); + ArrowBufferReset(&private->body_buffer); *emitted = 0; return NANOARROW_OK; } @@ -486,53 +488,44 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( ArrowErrorSet(error, "Arrow IPC files do not support replacement of dictionary ID %" PRId64, dictionary_id); + ArrowBufferReset(&private->buffer); + ArrowBufferReset(&private->body_buffer); return EINVAL; } - struct ArrowBuffer metadata_copy; - struct ArrowBuffer body_copy; - ArrowBufferInit(&metadata_copy); - ArrowBufferInit(&body_copy); - ArrowErrorCode result = - ArrowBufferAppend(&metadata_copy, private->buffer.data, private->buffer.size_bytes); - if (result == NANOARROW_OK) { - result = ArrowBufferAppend(&body_copy, private->body_buffer.data, - private->body_buffer.size_bytes); - } - - if (result != NANOARROW_OK) { - ArrowBufferReset(&metadata_copy); - ArrowBufferReset(&body_copy); - return result; - } - + int cached_was_added = 0; if (cached == NULL) { struct ArrowIpcWriterDictionaryCacheEntry new_entry = { .dictionary_id = dictionary_id, }; ArrowBufferInit(&new_entry.metadata); ArrowBufferInit(&new_entry.body); - result = ArrowBufferAppend(&private->dictionary_cache, &new_entry, sizeof(new_entry)); + ArrowErrorCode result = + ArrowBufferAppend(&private->dictionary_cache, &new_entry, sizeof(new_entry)); if (result != NANOARROW_OK) { - ArrowBufferReset(&metadata_copy); - ArrowBufferReset(&body_copy); return result; } cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); NANOARROW_DCHECK(cached != NULL); + cached_was_added = 1; } - result = ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); + ArrowErrorCode result = ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); if (result != NANOARROW_OK) { - ArrowBufferReset(&metadata_copy); - ArrowBufferReset(&body_copy); + if (cached_was_added) { + NANOARROW_ASSERT_OK(ArrowBufferResize( + &private->dictionary_cache, + private->dictionary_cache.size_bytes - + (int64_t)sizeof(struct ArrowIpcWriterDictionaryCacheEntry), + /*shrink_to_fit=*/0)); + } return result; } ArrowBufferReset(&cached->metadata); ArrowBufferReset(&cached->body); - ArrowBufferMove(&metadata_copy, &cached->metadata); - ArrowBufferMove(&body_copy, &cached->body); + ArrowBufferMove(&private->buffer, &cached->metadata); + ArrowBufferMove(&private->body_buffer, &cached->body); *emitted = 1; return NANOARROW_OK; } From ba617e32e46d43ebc3803804d1ba99b14f12ebf1 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 2 Sep 2026 14:12:14 -0400 Subject: [PATCH 10/16] test(ipc): finish dictionary writer coverage --- src/nanoarrow/ipc/encoder_test.cc | 37 +++++++++---- src/nanoarrow/ipc/writer.c | 27 +++++----- src/nanoarrow/ipc/writer_test.cc | 87 ++++++++++++++----------------- 3 files changed, 79 insertions(+), 72 deletions(-) diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index 3fdee9ff3..267e872d1 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -328,16 +328,15 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderSchemaMessageMetadata) { TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryReplacementFeature) { nanoarrow::UniqueSchema schema; - ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), - NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), NANOARROW_OK); ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "dict_col"), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); - ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0]->dictionary, - NANOARROW_TYPE_STRING), - NANOARROW_OK); + ASSERT_EQ( + ArrowSchemaInitFromType(schema->children[0]->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); nanoarrow::ipc::UniqueEncoder encoder; ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); @@ -364,8 +363,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderDictionaryReplacementFeature) { TEST(NanoarrowIpcTest, NanoarrowIpcEncoderNestedDictionaryValueSchema) { nanoarrow::UniqueSchema schema; - ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), - NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); struct ArrowSchema* dictionary_field = schema->children[0]; @@ -375,8 +373,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderNestedDictionaryValueSchema) { ASSERT_EQ(ArrowSchemaAllocateDictionary(dictionary_field), NANOARROW_OK); struct ArrowSchema* value_schema = dictionary_field->dictionary; - ASSERT_EQ(ArrowSchemaInitFromType(value_schema, NANOARROW_TYPE_STRUCT), - NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(value_schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(value_schema, 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(value_schema->children[0], NANOARROW_TYPE_STRING), NANOARROW_OK); @@ -1679,4 +1676,26 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderCompressorWithoutOutput) { EIO); EXPECT_THAT(error.message, ::testing::StartsWith("Compressor produced no output for a buffer of")); +TEST(NanoarrowIpcTest, NanoarrowIpcEncoderRejectsNestedDictionaryBatch) { + nanoarrow::UniqueSchema schema; + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_INT32), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(schema.get()), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); + + nanoarrow::UniqueArrayView array_view; + struct ArrowError error; + ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), &error), + NANOARROW_OK) + << error.message; + + nanoarrow::ipc::UniqueEncoder encoder; + ASSERT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); + nanoarrow::UniqueBuffer body; + EXPECT_EQ(ArrowIpcEncoderEncodeSimpleDictionaryBatch(encoder.get(), /*dictionary_id=*/0, + /*is_delta=*/0, array_view.get(), + body.get(), &error), + EINVAL); + EXPECT_STREQ(error.message, + "DictionaryBatch values array must not itself be dictionary-encoded"); } diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 146d7ef27..c9fcdcecc 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -201,8 +201,7 @@ struct ArrowIpcWriterDictionaryView { int force_emit; }; -static void ArrowIpcWriterResetDictionaryCache( - struct ArrowIpcWriterPrivate* private) { +static void ArrowIpcWriterResetDictionaryCache(struct ArrowIpcWriterPrivate* private) { int64_t n_cached_dictionaries = private->dictionary_cache.size_bytes / (int64_t)sizeof(struct ArrowIpcWriterDictionaryCacheEntry); @@ -319,10 +318,10 @@ ArrowErrorCode ArrowIpcWriterWriteSchema(struct ArrowIpcWriter* writer, ArrowIpcDictionaryEncodingsInit(&private->footer.dictionaries); NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowSchemaDeepCopy(in, &private->footer.schema), error); - NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowIpcDictionaryEncodingsAppendSchema( - &private->footer.dictionaries, - &private->footer.schema), - error); + NANOARROW_RETURN_NOT_OK_WITH_ERROR( + ArrowIpcDictionaryEncodingsAppendSchema(&private->footer.dictionaries, + &private->footer.schema), + error); } private->bytes_written += private->buffer.size_bytes; @@ -415,8 +414,8 @@ ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( if (private->writing_file && !is_delta) { int emitted; - return ArrowIpcWriterWriteDictionaryBatchIfChanged( - writer, dictionary_id, values_view, /*force_emit=*/0, &emitted, error); + return ArrowIpcWriterWriteDictionaryBatchIfChanged(writer, dictionary_id, values_view, + /*force_emit=*/0, &emitted, error); } NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); @@ -572,8 +571,7 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( ArrowBufferInit(&dictionaries); int64_t next_id = 0; ArrowErrorCode result = ArrowIpcWriterCollectDictionariesForArrayView( - array_view, &dictionaries, &next_id, - NANOARROW_IPC_NO_PARENT_DICTIONARY_ID); + array_view, &dictionaries, &next_id, NANOARROW_IPC_NO_PARENT_DICTIONARY_ID); if (result == NANOARROW_OK) { struct ArrowIpcWriterDictionaryView* dictionary_views = @@ -587,9 +585,8 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( for (int64_t i = n_dictionaries - 1; i >= 0; i--) { int emitted = 0; result = ArrowIpcWriterWriteDictionaryBatchIfChanged( - writer, dictionary_views[i].dictionary_id, - dictionary_views[i].values_view, dictionary_views[i].force_emit, - &emitted, error); + writer, dictionary_views[i].dictionary_id, dictionary_views[i].values_view, + dictionary_views[i].force_emit, &emitted, error); if (result != NANOARROW_OK) { break; } @@ -624,8 +621,8 @@ static ArrowErrorCode ArrowIpcWriterWriteArrayStreamImpl( NANOARROW_RETURN_NOT_OK(ArrowArrayViewSetArray(array_view, array, error)); - NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteDictionariesForArrayView( - writer, array_view, error)); + NANOARROW_RETURN_NOT_OK( + ArrowIpcWriterWriteDictionariesForArrayView(writer, array_view, error)); NANOARROW_RETURN_NOT_OK(ArrowIpcWriterWriteArrayView(writer, array_view, error)); ArrowArrayRelease(array); diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 4a1f578f7..0c1e58ed2 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -310,16 +310,15 @@ TEST(NanoarrowIpcWriter, WriteDictionaryBatch) { TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { struct ArrowError error; nanoarrow::UniqueSchema schema; - ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), - NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema.get(), 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), NANOARROW_OK); ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "dict_col"), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); - ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0]->dictionary, - NANOARROW_TYPE_STRING), - NANOARROW_OK); + ASSERT_EQ( + ArrowSchemaInitFromType(schema->children[0]->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); nanoarrow::UniqueSchema values_schema; ASSERT_EQ(ArrowSchemaInitFromType(values_schema.get(), NANOARROW_TYPE_STRING), @@ -347,19 +346,17 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { ASSERT_EQ(ArrowArrayInitFromSchema(batch2.get(), schema.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowArrayStartAppending(batch1.get()), NANOARROW_OK); ASSERT_EQ(ArrowArrayStartAppending(batch2.get()), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(batch1->children[0]->dictionary, - ArrowCharView("zero")), - NANOARROW_OK); + ASSERT_EQ( + ArrowArrayAppendString(batch1->children[0]->dictionary, ArrowCharView("zero")), + NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(batch1->children[0], 0), NANOARROW_OK); batch1->length = 1; - ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, - ArrowCharView("zero")), - NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, - ArrowCharView("one")), + ASSERT_EQ( + ArrowArrayAppendString(batch2->children[0]->dictionary, ArrowCharView("zero")), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, ArrowCharView("one")), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, - ArrowCharView("two")), + ASSERT_EQ(ArrowArrayAppendString(batch2->children[0]->dictionary, ArrowCharView("two")), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(batch2->children[0], 2), NANOARROW_OK); batch2->length = 1; @@ -370,12 +367,12 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { nanoarrow::UniqueArrayView delta_values_view; nanoarrow::UniqueArrayView batch1_view; nanoarrow::UniqueArrayView batch2_view; - ASSERT_EQ(ArrowArrayViewInitFromSchema(full_values_view.get(), values_schema.get(), - &error), - NANOARROW_OK); - ASSERT_EQ(ArrowArrayViewInitFromSchema(delta_values_view.get(), values_schema.get(), - &error), - NANOARROW_OK); + ASSERT_EQ( + ArrowArrayViewInitFromSchema(full_values_view.get(), values_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ( + ArrowArrayViewInitFromSchema(delta_values_view.get(), values_schema.get(), &error), + NANOARROW_OK); ASSERT_EQ(ArrowArrayViewInitFromSchema(batch1_view.get(), schema.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowArrayViewInitFromSchema(batch2_view.get(), schema.get(), &error), @@ -395,13 +392,13 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { nanoarrow::ipc::UniqueWriter writer; ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteSchema(writer.get(), schema.get(), &error), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( - writer.get(), 0, /*is_delta=*/0, full_values_view.get(), &error), + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(writer.get(), 0, /*is_delta=*/0, + full_values_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(writer.get(), batch1_view.get(), &error), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( - writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(writer.get(), 0, /*is_delta=*/1, + delta_values_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(writer.get(), batch2_view.get(), &error), NANOARROW_OK); @@ -435,13 +432,13 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { ASSERT_EQ(ArrowIpcWriterStartFile(file_writer.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteSchema(file_writer.get(), schema.get(), &error), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( - file_writer.get(), 0, /*is_delta=*/0, full_values_view.get(), &error), + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(file_writer.get(), 0, /*is_delta=*/0, + full_values_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch1_view.get(), &error), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch( - file_writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), + ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(file_writer.get(), 0, /*is_delta=*/1, + delta_values_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch2_view.get(), &error), NANOARROW_OK); @@ -677,8 +674,7 @@ TEST(NanoarrowIpcWriter, ReemitsParentWhenNestedDictionaryChanges) { NANOARROW_OK) << error.message; - EXPECT_EQ(DecodeDictionaryIds(output.get()), - (std::vector{1, 0, 1, 0})); + EXPECT_EQ(DecodeDictionaryIds(output.get()), (std::vector{1, 0, 1, 0})); struct ArrowIpcInputStream input; ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); @@ -826,13 +822,13 @@ TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { nanoarrow::UniqueArrayView roundtrip_view1; nanoarrow::UniqueArrayView roundtrip_view2; - ASSERT_EQ(ArrowArrayViewInitFromSchema(roundtrip_view1.get(), roundtrip_schema.get(), - &error), - NANOARROW_OK) + ASSERT_EQ( + ArrowArrayViewInitFromSchema(roundtrip_view1.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) << error.message; - ASSERT_EQ(ArrowArrayViewInitFromSchema(roundtrip_view2.get(), roundtrip_schema.get(), - &error), - NANOARROW_OK) + ASSERT_EQ( + ArrowArrayViewInitFromSchema(roundtrip_view2.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) << error.message; ASSERT_EQ(ArrowArrayViewSetArray(roundtrip_view1.get(), roundtrip_array1.get(), &error), NANOARROW_OK) @@ -845,13 +841,11 @@ TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { ArrowArrayViewGetStringUnsafe(roundtrip_view1->children[0]->dictionary, 1); struct ArrowStringView replacement_dictionary_value = ArrowArrayViewGetStringUnsafe(roundtrip_view2->children[0]->dictionary, 1); - EXPECT_EQ(std::string(first_dictionary_value.data, - first_dictionary_value.size_bytes), + EXPECT_EQ(std::string(first_dictionary_value.data, first_dictionary_value.size_bytes), "bar"); EXPECT_EQ(std::string(replacement_dictionary_value.data, replacement_dictionary_value.size_bytes), "baz"); - } TEST(NanoarrowIpcWriter, RejectsChangedDictionaryInFile) { @@ -874,8 +868,7 @@ TEST(NanoarrowIpcWriter, RejectsChangedDictionaryInFile) { ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); nanoarrow::ipc::UniqueWriter writer; ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) - << error.message; + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) << error.message; EXPECT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), EINVAL); @@ -905,15 +898,13 @@ TEST(NanoarrowIpcWriter, WritesDeltaDictionaryInFile) { ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); nanoarrow::ipc::UniqueWriter writer; ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) - << error.message; + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) << error.message; - EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch( - writer.get(), 0, /*is_delta=*/1, values_view.get(), &error), + EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch(writer.get(), 0, /*is_delta=*/1, + values_view.get(), &error), NANOARROW_OK) << error.message; - auto* private_data = - static_cast(writer->private_data); + auto* private_data = static_cast(writer->private_data); EXPECT_EQ(private_data->footer.dictionary_blocks.size_bytes, sizeof(struct ArrowIpcFileBlock)); } From 36a53f039fe53035eb17507b1e0d4503964bad8b Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 2 Sep 2026 14:10:22 -0400 Subject: [PATCH 11/16] feat(ipc): decode dictionary deltas --- src/nanoarrow/common/utils.c | 111 ++++++++++ src/nanoarrow/ipc/decoder.c | 130 +++++++++++- src/nanoarrow/ipc/decoder_test.cc | 338 +++++++++++++++++++++++++++++- 3 files changed, 565 insertions(+), 14 deletions(-) diff --git a/src/nanoarrow/common/utils.c b/src/nanoarrow/common/utils.c index c86d00b38..27267054a 100644 --- a/src/nanoarrow/common/utils.c +++ b/src/nanoarrow/common/utils.c @@ -47,6 +47,11 @@ #include "nanoarrow/nanoarrow.h" +#ifdef NANOARROW_NAMESPACE +#define ArrowArrayInternalTryUnshare \ + NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowArrayInternalTryUnshare) +#endif + const char* ArrowNanoarrowVersion(void) { return NANOARROW_VERSION; } int ArrowNanoarrowVersionInt(void) { return NANOARROW_VERSION_INT; } @@ -518,6 +523,112 @@ ArrowErrorCode ArrowSharedArrayBuffer(struct ArrowSharedArray* shared, int64_t i return NANOARROW_OK; } +// ArrowArrayMoveShared() wraps each node's buffers in references to the original +// array. When no clones of those references exist, recover the original mutable +// array so callers can continue appending without copying its contents. +static int ArrowArrayInternalCanUnshare(struct ArrowArray* array) { + if (!ArrowArrayIsInternal(array)) { + return 0; + } + + if (array->n_buffers > 0) { + struct ArrowSharedArrayPrivate* private_data = NULL; + for (int64_t i = 0; i < array->n_buffers; i++) { + struct ArrowBuffer* buffer = ArrowArrayBuffer(array, i); + if (buffer->allocator.free != &ArrowSharedArrayBufferFree) { + return 0; + } + + if (private_data == NULL) { + private_data = (struct ArrowSharedArrayPrivate*)buffer->allocator.private_data; + } else if (private_data != buffer->allocator.private_data) { + return 0; + } + } + + if (private_data == NULL || + ArrowSharedArrayUpdate(private_data, 0) != array->n_buffers || + !ArrowArrayIsInternal(&private_data->src) || + private_data->src.n_buffers != array->n_buffers) { + return 0; + } + + // Appending requires reallocatable buffers. Arrays decoded as zero-copy views + // use a deallocator-only allocator and must take the copying path once before + // they can be recovered here. + for (int64_t i = 0; i < private_data->src.n_buffers; i++) { + if (ArrowArrayBuffer(&private_data->src, i)->allocator.reallocate != + &ArrowBufferAllocatorMallocReallocate) { + return 0; + } + } + } + + for (int64_t i = 0; i < array->n_children; i++) { + if (!ArrowArrayInternalCanUnshare(array->children[i])) { + return 0; + } + } + + return array->dictionary == NULL || ArrowArrayInternalCanUnshare(array->dictionary); +} + +static void ArrowArrayInternalUnshare(struct ArrowArray* array) { + for (int64_t i = 0; i < array->n_children; i++) { + ArrowArrayInternalUnshare(array->children[i]); + } + + if (array->dictionary != NULL) { + ArrowArrayInternalUnshare(array->dictionary); + } + + // Nodes without buffers (e.g., run-end encoded arrays) were never wrapped in an + // ArrowSharedArrayPrivate and are already mutable once their children are mutable. + if (array->n_buffers == 0) { + return; + } + + struct ArrowSharedArrayPrivate* private_data = + (struct ArrowSharedArrayPrivate*)ArrowArrayBuffer(array, 0)->allocator.private_data; + struct ArrowArray mutable_array; + ArrowArrayMove(&private_data->src, &mutable_array); + + // Detach the wrapper buffers without decrementing the now-exclusive owner. The + // original buffers have been moved into mutable_array and will own their storage. + for (int64_t i = 0; i < array->n_buffers; i++) { + ArrowBufferInit(ArrowArrayBuffer(array, i)); + } + ArrowFree(private_data); + + // Children were shared independently. Move their recovered mutable arrays into + // the original child slots before releasing the wrapper shell. + for (int64_t i = 0; i < array->n_children; i++) { + ArrowArrayMove(array->children[i], mutable_array.children[i]); + } + if (array->dictionary != NULL) { + ArrowArrayMove(array->dictionary, mutable_array.dictionary); + } + + ArrowArrayRelease(array); + ArrowArrayMove(&mutable_array, array); +} + +#ifdef __cplusplus +extern "C" { +#endif + +NANOARROW_DLL int ArrowArrayInternalTryUnshare(struct ArrowArray* array) { + if (!ArrowArrayInternalCanUnshare(array)) { + return 0; + } + + ArrowArrayInternalUnshare(array); + return 1; +} +#ifdef __cplusplus +} +#endif + static const int kInt32DecimalDigits = 9; static const uint64_t kUInt32PowersOfTen[] = { diff --git a/src/nanoarrow/ipc/decoder.c b/src/nanoarrow/ipc/decoder.c index 9826b7591..3bb55b4db 100644 --- a/src/nanoarrow/ipc/decoder.c +++ b/src/nanoarrow/ipc/decoder.c @@ -36,6 +36,21 @@ #define NANOARROW_IPC_NO_DICTIONARY_ID INT64_MIN #define ns(x) FLATBUFFERS_WRAP_NAMESPACE(org_apache_arrow_flatbuf, x) +#ifdef NANOARROW_NAMESPACE +#define ArrowArrayInternalTryUnshare \ + NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowArrayInternalTryUnshare) +#endif + +// Internal common utility: recover a mutable ArrowArray after ArrowArrayMoveShared() +// when no clones still reference it. +#ifdef __cplusplus +extern "C" { +#endif + +NANOARROW_DLL int ArrowArrayInternalTryUnshare(struct ArrowArray* array); +#ifdef __cplusplus +} +#endif // Internal representation of a parsed "Field" from flatbuffers. This // represents a field in a depth-first walk of column arrays and their @@ -337,17 +352,117 @@ static ArrowErrorCode ArrowIpcDictionaryReplace(struct ArrowIpcDictionary* dicti return NANOARROW_OK; } +static ArrowErrorCode ArrowIpcArraySetDictionaries(struct ArrowArray* dst, + const struct ArrowArray* src) { + if (src->dictionary != NULL) { + NANOARROW_DCHECK(dst->dictionary != NULL); + if (dst->dictionary->release != NULL) { + ArrowArrayRelease(dst->dictionary); + } + NANOARROW_RETURN_NOT_OK(ArrowArrayCloneShared(src->dictionary, dst->dictionary)); + } + + for (int64_t i = 0; i < src->n_children; i++) { + NANOARROW_RETURN_NOT_OK( + ArrowIpcArraySetDictionaries(dst->children[i], src->children[i])); + } + return NANOARROW_OK; +} + +static void ArrowIpcArrayPrepareForAppend(struct ArrowArray* array, + const struct ArrowArrayView* array_view) { + // Finishing a view array materializes its variadic-buffer sizes. Appending may + // extend the last variadic buffer or add another one, so force the sizes buffer + // to be regenerated by the next ArrowArrayFinishBuildingDefault(). + if (array_view->storage_type == NANOARROW_TYPE_BINARY_VIEW || + array_view->storage_type == NANOARROW_TYPE_STRING_VIEW) { + ArrowBufferReset(ArrowArrayBuffer(array, array->n_buffers - 1)); + } + + for (int64_t i = 0; i < array->n_children; i++) { + ArrowIpcArrayPrepareForAppend(array->children[i], array_view->children[i]); + } +} + static ArrowErrorCode ArrowIpcDictionaryAppend(struct ArrowIpcDictionary* dictionary, struct ArrowArray* value, + struct ArrowArrayView* array_view, struct ArrowError* error) { - if (dictionary->current_value.release != NULL && - dictionary->current_value.length != 0) { - ArrowErrorSet(error, "Dictionary concatenation is not yet supported"); - return ENOTSUP; + if (dictionary->current_value.release == NULL || + dictionary->current_value.length == 0) { + return ArrowIpcDictionaryReplace(dictionary, value, error); + } + + // In the usual streaming loop, the previously returned batch has been released + // before the next one is requested. Recover the mutable backing array and append + // directly so a sequence of small deltas grows geometrically instead of copying + // the complete dictionary for every message. If an older batch is still alive, + // keep the copy-on-write path below to preserve its dictionary snapshot. + if (ArrowArrayInternalTryUnshare(&dictionary->current_value)) { + struct ArrowArray combined; + ArrowArrayMove(&dictionary->current_value, &combined); + + ArrowIpcArrayPrepareForAppend(&combined, array_view); + ArrowErrorCode result = ArrowArrayReserve(&combined, value->length); + if (result == NANOARROW_OK) { + result = ArrowArrayAppendArrayView(&combined, array_view, error); + } + if (result == NANOARROW_OK) { + result = ArrowIpcArraySetDictionaries(&combined, value); + } + if (result == NANOARROW_OK) { + result = ArrowArrayFinishBuildingDefault(&combined, error); + } + if (result == NANOARROW_OK) { + result = ArrowIpcDictionaryReplace(dictionary, &combined, error); + } + + if (combined.release != NULL) { + ArrowArrayRelease(&combined); + } + if (result == NANOARROW_OK) { + ArrowArrayRelease(value); + } + return result; } - NANOARROW_RETURN_NOT_OK(ArrowIpcDictionaryReplace(dictionary, value, error)); - return NANOARROW_OK; + struct ArrowArray combined; + combined.release = NULL; + NANOARROW_RETURN_NOT_OK(ArrowArrayInitFromArrayView(&combined, array_view, error)); + ArrowErrorCode result = ArrowArrayStartAppending(&combined); + if (result == NANOARROW_OK) { + result = + ArrowArrayReserve(&combined, dictionary->current_value.length + value->length); + } + if (result == NANOARROW_OK) { + result = ArrowArrayViewSetArray(array_view, &dictionary->current_value, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayAppendArrayView(&combined, array_view, error); + } + if (result == NANOARROW_OK) { + result = ArrowIpcArraySetDictionaries(&combined, value); + } + if (result == NANOARROW_OK) { + result = ArrowArrayViewSetArray(array_view, value, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayAppendArrayView(&combined, array_view, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayFinishBuildingDefault(&combined, error); + } + if (result == NANOARROW_OK) { + result = ArrowIpcDictionaryReplace(dictionary, &combined, error); + } + + if (combined.release != NULL) { + ArrowArrayRelease(&combined); + } + if (result == NANOARROW_OK) { + ArrowArrayRelease(value); + } + return result; } static void ArrowIpcDictionaryReset(struct ArrowIpcDictionary* dictionary) { @@ -2770,6 +2885,7 @@ static ArrowErrorCode ArrowIpcDecoderDecodeDictionaryInternal( struct ArrowIpcDecoderPrivate* dictionary_decoder_private_data = (struct ArrowIpcDecoderPrivate*)dictionary->decoder.private_data; dictionary->decoder.message_type = NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH; + dictionary->decoder.metadata_version = decoder->metadata_version; dictionary_decoder_private_data->last_message = record_batch; // Transfer the endianness and compression settings so that buffers are byte-swapped // and decompressed if needed (the nested decoder uses a default decompressor) @@ -2794,7 +2910,7 @@ static ArrowErrorCode ArrowIpcDecoderDecodeDictionaryInternal( } if (decoder->dictionary->is_delta) { - result = ArrowIpcDictionaryAppend(dictionary, &tmp, error); + result = ArrowIpcDictionaryAppend(dictionary, &tmp, array_view, error); } else { result = ArrowIpcDictionaryReplace(dictionary, &tmp, error); } diff --git a/src/nanoarrow/ipc/decoder_test.cc b/src/nanoarrow/ipc/decoder_test.cc index dcdde1761..f7f290002 100644 --- a/src/nanoarrow/ipc/decoder_test.cc +++ b/src/nanoarrow/ipc/decoder_test.cc @@ -20,10 +20,15 @@ #if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) #include +#include +#include +#include +#include #include #include #include #include +#include #include #endif #include @@ -802,12 +807,64 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { ASSERT_EQ(ArrowArrayViewGetStringUnsafe(&array_view, 1), "one"_asv); ASSERT_EQ(ArrowArrayViewGetStringUnsafe(&array_view, 2), "two"_asv); - // If we try to decode a delta dictionary, we should fail with a reasonable message + // A delta dictionary appends its values to the current dictionary const_cast(decoder.dictionary)->is_delta = 1; ASSERT_EQ(ArrowIpcDecoderDecodeDictionary( &decoder, body, NANOARROW_VALIDATION_LEVEL_FULL, &dictionaries, &error), - ENOTSUP); - ASSERT_STREQ(error.message, "Dictionary concatenation is not yet supported"); + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcDictionariesFindCurrentValue(&dictionaries, 0, &dictionary_value, &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(&array_view, dictionary_value, &error), NANOARROW_OK); + ASSERT_EQ(array_view.length, 6); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(&array_view, 0), "zero"_asv); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(&array_view, 3), "zero"_asv); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(&array_view, 5), "two"_asv); + + // Without live clones, repeated deltas reuse the mutable backing allocation. + ASSERT_EQ(ArrowIpcDecoderDecodeDictionary( + &decoder, body, NANOARROW_VALIDATION_LEVEL_FULL, &dictionaries, &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcDictionariesFindCurrentValue(&dictionaries, 0, &dictionary_value, &error), + NANOARROW_OK); + EXPECT_EQ(dictionary_value->length, 9); + const void* values_buffer_before = dictionary_value->buffers[2]; + ASSERT_EQ(ArrowIpcDecoderDecodeDictionary( + &decoder, body, NANOARROW_VALIDATION_LEVEL_FULL, &dictionaries, &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcDictionariesFindCurrentValue(&dictionaries, 0, &dictionary_value, &error), + NANOARROW_OK); + EXPECT_EQ(dictionary_value->length, 12); + EXPECT_EQ(dictionary_value->buffers[2], values_buffer_before); + + // Holding a clone forces copy-on-write and preserves the older snapshot. + struct ArrowArray snapshot; + ASSERT_EQ( + ArrowArrayCloneShared(const_cast(dictionary_value), &snapshot), + NANOARROW_OK); + const void* snapshot_values_buffer = snapshot.buffers[2]; + ASSERT_EQ(ArrowIpcDecoderDecodeDictionary( + &decoder, body, NANOARROW_VALIDATION_LEVEL_FULL, &dictionaries, &error), + NANOARROW_OK) + << error.message; + ASSERT_EQ( + ArrowIpcDictionariesFindCurrentValue(&dictionaries, 0, &dictionary_value, &error), + NANOARROW_OK); + EXPECT_EQ(dictionary_value->length, 15); + EXPECT_NE(dictionary_value->buffers[2], snapshot_values_buffer); + EXPECT_EQ(snapshot.length, 12); + + struct ArrowArrayView snapshot_view; + ArrowArrayViewInitFromType(&snapshot_view, NANOARROW_TYPE_STRING); + ASSERT_EQ(ArrowArrayViewSetArray(&snapshot_view, &snapshot, &error), NANOARROW_OK); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(&snapshot_view, 11), "two"_asv); + ArrowArrayViewReset(&snapshot_view); + ArrowArrayRelease(&snapshot); // After all of this, we should be able to actually decode a RecordBatch ASSERT_EQ(ArrowIpcDecoderSetSchemaWithDictionaries(&decoder, &schema, @@ -827,7 +884,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { << error.message; ASSERT_NE(batch_view->children[0]->dictionary, nullptr); - ASSERT_EQ(batch_view->children[0]->dictionary->length, 3); + ASSERT_EQ(batch_view->children[0]->dictionary->length, 15); ASSERT_EQ(ArrowArrayViewGetStringUnsafe(batch_view->children[0]->dictionary, 0), "zero"_asv); @@ -839,7 +896,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { << error.message; ASSERT_NE(column_view->dictionary, nullptr); - ASSERT_EQ(column_view->dictionary->length, 3); + ASSERT_EQ(column_view->dictionary->length, 15); ASSERT_EQ(ArrowArrayViewGetStringUnsafe(column_view->dictionary, 0), "zero"_asv); // Decode the array from the ArrowBufferView @@ -850,7 +907,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { NANOARROW_OK) << error.message; ASSERT_NE(batch.children[0]->dictionary, nullptr); - ASSERT_EQ(batch.children[0]->dictionary->length, 3); + ASSERT_EQ(batch.children[0]->dictionary->length, 15); ArrowArrayRelease(&batch); // Decode the array from a shared buffer @@ -868,7 +925,7 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { NANOARROW_OK) << error.message; ASSERT_NE(batch.children[0]->dictionary, nullptr); - ASSERT_EQ(batch.children[0]->dictionary->length, 3); + ASSERT_EQ(batch.children[0]->dictionary->length, 15); ArrowArrayRelease(&batch); ArrowBufferReset(&record_batch_shared); @@ -2099,4 +2156,271 @@ INSTANTIATE_TEST_SUITE_P(NanoarrowIpcTest, ArrowTypeIdParameterizedTestFixture, NANOARROW_TYPE_DECIMAL128, NANOARROW_TYPE_DECIMAL256, NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO)); + +enum class DeltaDictionaryValueCase { + kBoolean, + kInt64, + kInt64WithNull, + kUInt64, + kDouble, + kString, + kBinary, + kDecimal128, + kList, + kStruct, + kFixedSizeList +}; + +class DeltaDictionaryTypeTest + : public ::testing::TestWithParam {}; + +static std::shared_ptr FinishDeltaBuilder(arrow::ArrayBuilder* builder) { + std::shared_ptr out; + EXPECT_TRUE(builder->Finish(&out).ok()); + return out; +} + +static std::shared_ptr MakeDeltaDictionaryValues( + DeltaDictionaryValueCase value_case) { + switch (value_case) { + case DeltaDictionaryValueCase::kBoolean: { + arrow::BooleanBuilder builder; + EXPECT_TRUE(builder.Append(true).ok()); + EXPECT_TRUE(builder.Append(false).ok()); + EXPECT_TRUE(builder.Append(true).ok()); + EXPECT_TRUE(builder.Append(false).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kInt64: + case DeltaDictionaryValueCase::kInt64WithNull: { + arrow::Int64Builder builder; + EXPECT_TRUE(builder.Append(1).ok()); + if (value_case == DeltaDictionaryValueCase::kInt64WithNull) { + EXPECT_TRUE(builder.AppendNull().ok()); + } else { + EXPECT_TRUE(builder.Append(-2).ok()); + } + EXPECT_TRUE(builder.Append(3).ok()); + EXPECT_TRUE(builder.Append(4).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kUInt64: { + arrow::UInt64Builder builder; + EXPECT_TRUE(builder.Append(1).ok()); + EXPECT_TRUE(builder.Append(2).ok()); + EXPECT_TRUE(builder.Append(3).ok()); + EXPECT_TRUE(builder.Append(4).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kDouble: { + arrow::DoubleBuilder builder; + EXPECT_TRUE(builder.Append(1.5).ok()); + EXPECT_TRUE(builder.Append(-2.25).ok()); + EXPECT_TRUE(builder.Append(3.5).ok()); + EXPECT_TRUE(builder.Append(4.75).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kString: { + arrow::StringBuilder builder; + EXPECT_TRUE(builder.Append("one", 3).ok()); + EXPECT_TRUE(builder.Append("two", 3).ok()); + EXPECT_TRUE(builder.Append("three", 5).ok()); + EXPECT_TRUE(builder.Append("four", 4).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kBinary: { + arrow::BinaryBuilder builder; + EXPECT_TRUE(builder.Append("one", 3).ok()); + EXPECT_TRUE(builder.Append("two", 3).ok()); + EXPECT_TRUE(builder.Append("three", 5).ok()); + EXPECT_TRUE(builder.Append("four", 4).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kDecimal128: { + arrow::Decimal128Builder builder(arrow::decimal128(10, 2)); + EXPECT_TRUE(builder.Append(arrow::Decimal128(125)).ok()); + EXPECT_TRUE(builder.Append(arrow::Decimal128(-250)).ok()); + EXPECT_TRUE(builder.Append(arrow::Decimal128(375)).ok()); + EXPECT_TRUE(builder.Append(arrow::Decimal128(400)).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kList: { + auto value_builder = std::make_shared(); + arrow::ListBuilder builder(arrow::default_memory_pool(), value_builder); + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(value_builder->Append(1).ok()); + EXPECT_TRUE(value_builder->Append(2).ok()); + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(value_builder->Append(3).ok()); + EXPECT_TRUE(value_builder->AppendNull().ok()); + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(value_builder->Append(4).ok()); + EXPECT_TRUE(value_builder->Append(5).ok()); + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kStruct: { + auto int_builder = std::make_shared(); + auto string_builder = std::make_shared(); + auto type = arrow::struct_( + {arrow::field("i", arrow::int32()), arrow::field("s", arrow::utf8())}); + arrow::StructBuilder builder(type, arrow::default_memory_pool(), + {int_builder, string_builder}); + for (int32_t i = 1; i <= 4; i++) { + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(int_builder->Append(i).ok()); + if (i == 3) { + EXPECT_TRUE(string_builder->AppendNull().ok()); + } else { + EXPECT_TRUE(string_builder->Append(std::to_string(i)).ok()); + } + } + return FinishDeltaBuilder(&builder); + } + case DeltaDictionaryValueCase::kFixedSizeList: { + auto value_builder = std::make_shared(); + arrow::FixedSizeListBuilder builder(arrow::default_memory_pool(), value_builder, 2); + for (int16_t value = 1; value <= 7; value += 2) { + EXPECT_TRUE(builder.Append().ok()); + EXPECT_TRUE(value_builder->Append(value).ok()); + EXPECT_TRUE(value_builder->Append(value + 1).ok()); + } + return FinishDeltaBuilder(&builder); + } + } + + ADD_FAILURE() << "Unknown dictionary value case"; + return nullptr; +} + +static std::shared_ptr MakeDeltaDictionaryIndices(bool extended) { + arrow::Int32Builder builder; + if (extended) { + EXPECT_TRUE(builder.Append(2).ok()); + EXPECT_TRUE(builder.Append(3).ok()); + EXPECT_TRUE(builder.Append(1).ok()); + EXPECT_TRUE(builder.AppendNull().ok()); + } else { + EXPECT_TRUE(builder.Append(0).ok()); + EXPECT_TRUE(builder.Append(1).ok()); + EXPECT_TRUE(builder.AppendNull().ok()); + EXPECT_TRUE(builder.Append(0).ok()); + } + return FinishDeltaBuilder(&builder); +} + +static void AssertReadsArrowCppDeltaStream( + const std::shared_ptr& dictionary_array1, + const std::shared_ptr& dictionary_array2) { + auto schema = arrow::schema({arrow::field("dictionary", dictionary_array1->type())}); + auto expected1 = arrow::RecordBatch::Make(schema, 4, {dictionary_array1}); + auto expected2 = arrow::RecordBatch::Make(schema, 4, {dictionary_array2}); + + auto maybe_sink = arrow::io::BufferOutputStream::Create(); + ASSERT_TRUE(maybe_sink.ok()) << maybe_sink.status(); + auto sink = maybe_sink.ValueUnsafe(); + auto options = arrow::ipc::IpcWriteOptions::Defaults(); + options.emit_dictionary_deltas = true; + auto maybe_writer = arrow::ipc::MakeStreamWriter(sink, schema, options); + ASSERT_TRUE(maybe_writer.ok()) << maybe_writer.status(); + auto writer = maybe_writer.ValueUnsafe(); + ASSERT_TRUE(writer->WriteRecordBatch(*expected1).ok()); + ASSERT_TRUE(writer->WriteRecordBatch(*expected2).ok()); + ASSERT_TRUE(writer->Close().ok()); + auto maybe_buffer = sink->Finish(); + ASSERT_TRUE(maybe_buffer.ok()) << maybe_buffer.status(); + auto buffer = maybe_buffer.ValueUnsafe(); + + nanoarrow::UniqueBuffer ipc_buffer; + ASSERT_EQ(ArrowBufferAppend(ipc_buffer.get(), buffer->data(), buffer->size()), + NANOARROW_OK); + struct ArrowIpcInputStream input; + ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, ipc_buffer.get()), NANOARROW_OK); + nanoarrow::UniqueArrayStream reader; + ASSERT_EQ(ArrowIpcArrayStreamReaderInit(reader.get(), &input, nullptr), NANOARROW_OK); + + struct ArrowError error; + nanoarrow::UniqueSchema roundtrip_schema; + ASSERT_EQ(ArrowArrayStreamGetSchema(reader.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) + << error.message; + auto maybe_arrow_schema = arrow::ImportSchema(roundtrip_schema.get()); + ASSERT_TRUE(maybe_arrow_schema.ok()) << maybe_arrow_schema.status(); + auto arrow_schema = maybe_arrow_schema.ValueUnsafe(); + + nanoarrow::UniqueArray roundtrip1; + nanoarrow::UniqueArray roundtrip2; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip1.get(), &error), NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip2.get(), &error), NANOARROW_OK) + << error.message; + auto maybe_roundtrip1 = arrow::ImportRecordBatch(roundtrip1.get(), arrow_schema); + auto maybe_roundtrip2 = arrow::ImportRecordBatch(roundtrip2.get(), arrow_schema); + ASSERT_TRUE(maybe_roundtrip1.ok()) << maybe_roundtrip1.status(); + ASSERT_TRUE(maybe_roundtrip2.ok()) << maybe_roundtrip2.status(); + EXPECT_TRUE(maybe_roundtrip1.ValueUnsafe()->Equals(*expected1)); + EXPECT_TRUE(maybe_roundtrip2.ValueUnsafe()->Equals(*expected2)); +} + +TEST_P(DeltaDictionaryTypeTest, ReadsArrowCppDeltaStream) { + auto values2 = MakeDeltaDictionaryValues(GetParam()); + ASSERT_NE(values2, nullptr); + auto values1 = values2->Slice(0, 2); + auto dictionary_type = arrow::dictionary(arrow::int32(), values2->type()); + auto maybe_array1 = arrow::DictionaryArray::FromArrays( + dictionary_type, MakeDeltaDictionaryIndices(false), values1); + auto maybe_array2 = arrow::DictionaryArray::FromArrays( + dictionary_type, MakeDeltaDictionaryIndices(true), values2); + ASSERT_TRUE(maybe_array1.ok()) << maybe_array1.status(); + ASSERT_TRUE(maybe_array2.ok()) << maybe_array2.status(); + AssertReadsArrowCppDeltaStream(maybe_array1.ValueUnsafe(), maybe_array2.ValueUnsafe()); +} + +TEST(NanoarrowIpcTest, ReadsArrowCppDenseUnionDictionaryDelta) { + arrow::Int8Builder type_ids_builder; + arrow::Int32Builder offsets_builder; + arrow::Int32Builder ints_builder; + arrow::StringBuilder strings_builder; + for (int8_t type_id : {5, 7, 5, 7}) { + EXPECT_TRUE(type_ids_builder.Append(type_id).ok()); + } + for (int32_t offset : {0, 0, 1, 1}) { + EXPECT_TRUE(offsets_builder.Append(offset).ok()); + } + EXPECT_TRUE(ints_builder.Append(1).ok()); + EXPECT_TRUE(ints_builder.Append(2).ok()); + EXPECT_TRUE(strings_builder.Append("a", 1).ok()); + EXPECT_TRUE(strings_builder.Append("b", 1).ok()); + + auto maybe_values2 = arrow::DenseUnionArray::Make( + *std::static_pointer_cast(FinishDeltaBuilder(&type_ids_builder)), + *std::static_pointer_cast(FinishDeltaBuilder(&offsets_builder)), + {FinishDeltaBuilder(&ints_builder), FinishDeltaBuilder(&strings_builder)}, + {"i", "s"}, {5, 7}); + ASSERT_TRUE(maybe_values2.ok()) << maybe_values2.status(); + auto values2 = maybe_values2.ValueUnsafe(); + auto values1 = values2->Slice(0, 2); + + auto dictionary_type = arrow::dictionary(arrow::int32(), values2->type()); + auto maybe_array1 = arrow::DictionaryArray::FromArrays( + dictionary_type, MakeDeltaDictionaryIndices(false), values1); + auto maybe_array2 = arrow::DictionaryArray::FromArrays( + dictionary_type, MakeDeltaDictionaryIndices(true), values2); + ASSERT_TRUE(maybe_array1.ok()) << maybe_array1.status(); + ASSERT_TRUE(maybe_array2.ok()) << maybe_array2.status(); + AssertReadsArrowCppDeltaStream(maybe_array1.ValueUnsafe(), maybe_array2.ValueUnsafe()); +} + +INSTANTIATE_TEST_SUITE_P(NanoarrowIpcDecoder, DeltaDictionaryTypeTest, + ::testing::Values(DeltaDictionaryValueCase::kBoolean, + DeltaDictionaryValueCase::kInt64, + DeltaDictionaryValueCase::kInt64WithNull, + DeltaDictionaryValueCase::kUInt64, + DeltaDictionaryValueCase::kDouble, + DeltaDictionaryValueCase::kString, + DeltaDictionaryValueCase::kBinary, + DeltaDictionaryValueCase::kDecimal128, + DeltaDictionaryValueCase::kList, + DeltaDictionaryValueCase::kStruct, + DeltaDictionaryValueCase::kFixedSizeList)); #endif From 4e76af4d7308e2b52e22df78f2716517fbad5ef7 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Tue, 1 Sep 2026 23:53:04 -0400 Subject: [PATCH 12/16] feat(ipc): emit append-only dictionaries as deltas --- src/nanoarrow/ipc/decoder.c | 6 +- src/nanoarrow/ipc/writer.c | 281 +++++++++++++++++++++++++------ src/nanoarrow/ipc/writer_test.cc | 256 +++++++++++++++++++++++++++- src/nanoarrow/nanoarrow_ipc.h | 4 + 4 files changed, 488 insertions(+), 59 deletions(-) diff --git a/src/nanoarrow/ipc/decoder.c b/src/nanoarrow/ipc/decoder.c index 3bb55b4db..ecf198e9d 100644 --- a/src/nanoarrow/ipc/decoder.c +++ b/src/nanoarrow/ipc/decoder.c @@ -405,7 +405,7 @@ static ArrowErrorCode ArrowIpcDictionaryAppend(struct ArrowIpcDictionary* dictio ArrowIpcArrayPrepareForAppend(&combined, array_view); ArrowErrorCode result = ArrowArrayReserve(&combined, value->length); if (result == NANOARROW_OK) { - result = ArrowArrayAppendArrayView(&combined, array_view, error); + result = ArrowArrayAppendStorageFromArrayView(&combined, array_view, error); } if (result == NANOARROW_OK) { result = ArrowIpcArraySetDictionaries(&combined, value); @@ -438,7 +438,7 @@ static ArrowErrorCode ArrowIpcDictionaryAppend(struct ArrowIpcDictionary* dictio result = ArrowArrayViewSetArray(array_view, &dictionary->current_value, error); } if (result == NANOARROW_OK) { - result = ArrowArrayAppendArrayView(&combined, array_view, error); + result = ArrowArrayAppendStorageFromArrayView(&combined, array_view, error); } if (result == NANOARROW_OK) { result = ArrowIpcArraySetDictionaries(&combined, value); @@ -447,7 +447,7 @@ static ArrowErrorCode ArrowIpcDictionaryAppend(struct ArrowIpcDictionary* dictio result = ArrowArrayViewSetArray(array_view, value, error); } if (result == NANOARROW_OK) { - result = ArrowArrayAppendArrayView(&combined, array_view, error); + result = ArrowArrayAppendStorageFromArrayView(&combined, array_view, error); } if (result == NANOARROW_OK) { result = ArrowArrayFinishBuildingDefault(&combined, error); diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index c9fcdcecc..593beb4fb 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -188,8 +188,7 @@ struct ArrowIpcWriterPrivate { struct ArrowIpcWriterDictionaryCacheEntry { int64_t dictionary_id; - struct ArrowBuffer metadata; - struct ArrowBuffer body; + struct ArrowArray values; }; #define NANOARROW_IPC_NO_PARENT_DICTIONARY_ID -1 @@ -208,8 +207,9 @@ static void ArrowIpcWriterResetDictionaryCache(struct ArrowIpcWriterPrivate* pri struct ArrowIpcWriterDictionaryCacheEntry* cached_dictionaries = (struct ArrowIpcWriterDictionaryCacheEntry*)private->dictionary_cache.data; for (int64_t i = 0; i < n_cached_dictionaries; i++) { - ArrowBufferReset(&cached_dictionaries[i].metadata); - ArrowBufferReset(&cached_dictionaries[i].body); + if (cached_dictionaries[i].values.release != NULL) { + ArrowArrayRelease(&cached_dictionaries[i].values); + } } ArrowBufferReset(&private->dictionary_cache); } @@ -402,8 +402,8 @@ static ArrowErrorCode ArrowIpcWriterWriteEncodedDictionaryBatch( static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( struct ArrowIpcWriter* writer, int64_t dictionary_id, - const struct ArrowArrayView* values_view, int force_emit, int* emitted, - struct ArrowError* error); + const struct ArrowArrayView* values_view, int force_emit, int allow_delta, + int* emitted, struct ArrowError* error); ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( struct ArrowIpcWriter* writer, int64_t dictionary_id, char is_delta, @@ -414,8 +414,9 @@ ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( if (private->writing_file && !is_delta) { int emitted; - return ArrowIpcWriterWriteDictionaryBatchIfChanged(writer, dictionary_id, values_view, - /*force_emit=*/0, &emitted, error); + return ArrowIpcWriterWriteDictionaryBatchIfChanged( + writer, dictionary_id, values_view, /*force_emit=*/0, + /*allow_delta=*/0, &emitted, error); } NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); @@ -432,12 +433,6 @@ ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( return ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); } -static int ArrowIpcWriterBufferEquals(const struct ArrowBuffer* lhs, - const struct ArrowBuffer* rhs) { - return lhs->size_bytes == rhs->size_bytes && - (lhs->size_bytes == 0 || memcmp(lhs->data, rhs->data, lhs->size_bytes) == 0); -} - static struct ArrowIpcWriterDictionaryCacheEntry* ArrowIpcWriterFindDictionaryCacheEntry( struct ArrowIpcWriterPrivate* private, int64_t dictionary_id) { int64_t n_cached_dictionaries = @@ -454,62 +449,229 @@ static struct ArrowIpcWriterDictionaryCacheEntry* ArrowIpcWriterFindDictionaryCa return NULL; } +static ArrowErrorCode ArrowIpcWriterArrayViewInitLike(struct ArrowArrayView* out, + const struct ArrowArrayView* src) { + ArrowArrayViewInitFromType(out, src->storage_type); + out->layout = src->layout; + + ArrowErrorCode result = ArrowArrayViewAllocateChildren(out, src->n_children); + if (result != NANOARROW_OK) { + ArrowArrayViewReset(out); + return result; + } + + for (int64_t i = 0; i < src->n_children; i++) { + result = ArrowIpcWriterArrayViewInitLike(out->children[i], src->children[i]); + if (result != NANOARROW_OK) { + ArrowArrayViewReset(out); + return result; + } + } + + if (src->dictionary != NULL) { + result = ArrowArrayViewAllocateDictionary(out); + if (result != NANOARROW_OK) { + ArrowArrayViewReset(out); + return result; + } + + result = ArrowIpcWriterArrayViewInitLike(out->dictionary, src->dictionary); + if (result != NANOARROW_OK) { + ArrowArrayViewReset(out); + return result; + } + } + + return NANOARROW_OK; +} + +static ArrowErrorCode ArrowIpcWriterMaterializeArrayView(const struct ArrowArrayView* src, + int64_t offset, int64_t length, + struct ArrowArray* out, + struct ArrowError* error) { + out->release = NULL; + if (offset < 0 || length < 0 || offset > src->length || length > src->length - offset) { + ArrowErrorSet(error, + "Invalid dictionary slice [%" PRId64 ", %" PRId64 + ") for array of length %" PRId64, + offset, offset + length, src->length); + return EINVAL; + } + + struct ArrowArrayView slice = *src; + slice.offset += offset; + slice.length = length; + slice.null_count = -1; + + ArrowErrorCode result = ArrowArrayInitFromArrayView(out, src, error); + if (result == NANOARROW_OK) { + result = ArrowArrayStartAppending(out); + } + if (result == NANOARROW_OK) { + result = ArrowArrayReserve(out, length); + } + if (result == NANOARROW_OK) { + result = ArrowArrayAppendStorageFromArrayView(out, &slice, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayFinishBuildingDefault(out, error); + } + + if (result != NANOARROW_OK && out->release != NULL) { + ArrowArrayRelease(out); + } + return result; +} + +static ArrowErrorCode ArrowIpcWriterCompareMaterializedArrays( + const struct ArrowArray* lhs, const struct ArrowArray* rhs, + const struct ArrowArrayView* shape, int* out, struct ArrowError* error) { + struct ArrowArrayView lhs_view; + struct ArrowArrayView rhs_view; + ArrowArrayViewInitFromType(&lhs_view, NANOARROW_TYPE_UNINITIALIZED); + ArrowArrayViewInitFromType(&rhs_view, NANOARROW_TYPE_UNINITIALIZED); + + ArrowErrorCode result = ArrowIpcWriterArrayViewInitLike(&lhs_view, shape); + if (result == NANOARROW_OK) { + result = ArrowIpcWriterArrayViewInitLike(&rhs_view, shape); + } + if (result == NANOARROW_OK) { + result = ArrowArrayViewSetArray(&lhs_view, lhs, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayViewSetArray(&rhs_view, rhs, error); + } + if (result == NANOARROW_OK) { + result = ArrowArrayViewCompare(&lhs_view, &rhs_view, NANOARROW_COMPARE_IDENTICAL, out, + NULL); + } + + ArrowArrayViewReset(&lhs_view); + ArrowArrayViewReset(&rhs_view); + return result; +} + +static ArrowErrorCode ArrowIpcWriterArrayViewSetMaterialized( + struct ArrowArrayView* out, const struct ArrowArrayView* shape, + const struct ArrowArray* array, struct ArrowError* error) { + ArrowArrayViewInitFromType(out, NANOARROW_TYPE_UNINITIALIZED); + NANOARROW_RETURN_NOT_OK(ArrowIpcWriterArrayViewInitLike(out, shape)); + ArrowErrorCode result = ArrowArrayViewSetArray(out, array, error); + if (result != NANOARROW_OK) { + ArrowArrayViewReset(out); + } + return result; +} + static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( struct ArrowIpcWriter* writer, int64_t dictionary_id, - const struct ArrowArrayView* values_view, int force_emit, int* emitted, - struct ArrowError* error) { + const struct ArrowArrayView* values_view, int force_emit, int allow_delta, + int* emitted, struct ArrowError* error) { struct ArrowIpcWriterPrivate* private = (struct ArrowIpcWriterPrivate*)writer->private_data; - NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); - NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); + struct ArrowArray current_values = {.release = NULL}; + struct ArrowArray prefix_values = {.release = NULL}; + struct ArrowArray delta_values = {.release = NULL}; + struct ArrowArrayView encoded_view; + ArrowArrayViewInitFromType(&encoded_view, NANOARROW_TYPE_UNINITIALIZED); - NANOARROW_RETURN_NOT_OK(ArrowIpcEncoderEncodeSimpleDictionaryBatch( - &private->encoder, dictionary_id, /*is_delta=*/0, values_view, - &private->body_buffer, error)); - NANOARROW_RETURN_NOT_OK_WITH_ERROR( - ArrowIpcEncoderFinalizeBuffer(&private->encoder, /*encapsulate=*/1, - &private->buffer), - error); + ArrowErrorCode result = ArrowIpcWriterMaterializeArrayView( + values_view, 0, values_view->length, ¤t_values, error); + if (result != NANOARROW_OK) { + return result; + } struct ArrowIpcWriterDictionaryCacheEntry* cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); - if (!force_emit && cached != NULL && - ArrowIpcWriterBufferEquals(&cached->metadata, &private->buffer) && - ArrowIpcWriterBufferEquals(&cached->body, &private->body_buffer)) { - ArrowBufferReset(&private->buffer); - ArrowBufferReset(&private->body_buffer); + int values_equal = 0; + int is_delta = 0; + int cached_was_added = 0; + const struct ArrowArray* values_to_encode = NULL; + if (cached != NULL) { + result = ArrowIpcWriterCompareMaterializedArrays(&cached->values, ¤t_values, + values_view, &values_equal, error); + if (result != NANOARROW_OK) { + goto cleanup; + } + } + + if (!force_emit && cached != NULL && values_equal) { *emitted = 0; - return NANOARROW_OK; + result = NANOARROW_OK; + goto cleanup; } - if (private->writing_file && cached != NULL) { + if (allow_delta && !force_emit && cached != NULL && + current_values.length > cached->values.length) { + result = ArrowIpcWriterMaterializeArrayView(values_view, 0, cached->values.length, + &prefix_values, error); + if (result != NANOARROW_OK) { + goto cleanup; + } + + int prefix_equal = 0; + result = ArrowIpcWriterCompareMaterializedArrays(&cached->values, &prefix_values, + values_view, &prefix_equal, error); + if (result != NANOARROW_OK) { + goto cleanup; + } + + if (prefix_equal) { + result = ArrowIpcWriterMaterializeArrayView( + values_view, cached->values.length, + current_values.length - cached->values.length, &delta_values, error); + if (result != NANOARROW_OK) { + goto cleanup; + } + is_delta = 1; + } + } + + if (private->writing_file && cached != NULL && !is_delta) { ArrowErrorSet(error, "Arrow IPC files do not support replacement of dictionary ID %" PRId64, dictionary_id); - ArrowBufferReset(&private->buffer); - ArrowBufferReset(&private->body_buffer); - return EINVAL; + result = EINVAL; + goto cleanup; + } + + values_to_encode = is_delta ? &delta_values : ¤t_values; + result = ArrowIpcWriterArrayViewSetMaterialized(&encoded_view, values_view, + values_to_encode, error); + if (result != NANOARROW_OK) { + goto cleanup; + } + + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->buffer, 0, 0)); + NANOARROW_ASSERT_OK(ArrowBufferResize(&private->body_buffer, 0, 0)); + + result = ArrowIpcEncoderEncodeSimpleDictionaryBatch(&private->encoder, dictionary_id, + is_delta, &encoded_view, + &private->body_buffer, error); + if (result == NANOARROW_OK) { + result = ArrowIpcEncoderFinalizeBuffer(&private->encoder, /*encapsulate=*/1, + &private->buffer); + } + if (result != NANOARROW_OK) { + goto cleanup; } - int cached_was_added = 0; if (cached == NULL) { struct ArrowIpcWriterDictionaryCacheEntry new_entry = { .dictionary_id = dictionary_id, + .values = {.release = NULL}, }; - ArrowBufferInit(&new_entry.metadata); - ArrowBufferInit(&new_entry.body); - ArrowErrorCode result = - ArrowBufferAppend(&private->dictionary_cache, &new_entry, sizeof(new_entry)); + result = ArrowBufferAppend(&private->dictionary_cache, &new_entry, sizeof(new_entry)); if (result != NANOARROW_OK) { - return result; + goto cleanup; } cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); NANOARROW_DCHECK(cached != NULL); cached_was_added = 1; } - ArrowErrorCode result = ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); + result = ArrowIpcWriterWriteEncodedDictionaryBatch(writer, error); if (result != NANOARROW_OK) { if (cached_was_added) { NANOARROW_ASSERT_OK(ArrowBufferResize( @@ -518,23 +680,35 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( (int64_t)sizeof(struct ArrowIpcWriterDictionaryCacheEntry), /*shrink_to_fit=*/0)); } - return result; + goto cleanup; } - ArrowBufferReset(&cached->metadata); - ArrowBufferReset(&cached->body); - ArrowBufferMove(&private->buffer, &cached->metadata); - ArrowBufferMove(&private->body_buffer, &cached->body); + if (cached->values.release != NULL) { + ArrowArrayRelease(&cached->values); + } + ArrowArrayMove(¤t_values, &cached->values); *emitted = 1; - return NANOARROW_OK; + +cleanup: + if (current_values.release != NULL) { + ArrowArrayRelease(¤t_values); + } + if (prefix_values.release != NULL) { + ArrowArrayRelease(&prefix_values); + } + if (delta_values.release != NULL) { + ArrowArrayRelease(&delta_values); + } + ArrowArrayViewReset(&encoded_view); + return result; } // Walk the array in the same depth-first order the schema encoder uses to assign // dictionary ids (see ArrowIpcDictionaryEncodingsAppendSchema): a dictionary-encoded // node claims the next id before descending into its children and then its values. -// Emit a full (non-delta) DictionaryBatch before the first RecordBatch and whenever -// the serialized dictionary changes. Each array in the input stream carries its own -// dictionary, but identical dictionaries do not need to be repeated in the IPC stream. +// Emit a full DictionaryBatch before the first RecordBatch. For later batches, suppress +// identical dictionaries, emit an append-only suffix as a delta, or emit a replacement +// in stream mode. Each array in the input stream carries its own dictionary. static ArrowErrorCode ArrowIpcWriterCollectDictionariesForArrayView( const struct ArrowArrayView* array_view, struct ArrowBuffer* dictionaries, int64_t* next_id, int64_t parent_dictionary_id) { @@ -585,8 +759,9 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( for (int64_t i = n_dictionaries - 1; i >= 0; i--) { int emitted = 0; result = ArrowIpcWriterWriteDictionaryBatchIfChanged( - writer, dictionary_views[i].dictionary_id, dictionary_views[i].values_view, - dictionary_views[i].force_emit, &emitted, error); + writer, dictionary_views[i].dictionary_id, + dictionary_views[i].values_view, dictionary_views[i].force_emit, + /*allow_delta=*/1, &emitted, error); if (result != NANOARROW_OK) { break; } diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 0c1e58ed2..e912faf14 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -440,6 +440,10 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(file_writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), NANOARROW_OK); + EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch( + file_writer.get(), 0, /*is_delta=*/0, + batch2_view->children[0]->dictionary, &error), + EINVAL); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch2_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), nullptr, &error), @@ -466,7 +470,8 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { static void MakeDictionaryStructArray(struct ArrowArray* array, struct ArrowSchema* schema, const char* value0 = "foo", - const char* value1 = "bar") { + const char* value1 = "bar", + const char* value2 = nullptr) { ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), @@ -484,15 +489,56 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value0)), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value1)), NANOARROW_OK); + if (value2 != nullptr) { + ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value2)), NANOARROW_OK); + } ASSERT_EQ(ArrowArrayAppendInt(indices, 0), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendInt(indices, 1), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(indices, value2 == nullptr ? 1 : 2), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendInt(indices, 0), NANOARROW_OK); array->length = 3; ASSERT_EQ(ArrowArrayFinishBuildingDefault(array, nullptr), NANOARROW_OK); } +static void MakeRunEndDictionaryStructArray(struct ArrowArray* array, + struct ArrowSchema* schema, + bool extended) { + struct ArrowError error; + ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "dict_col"), NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); + ArrowSchemaInit(schema->children[0]->dictionary); + ASSERT_EQ(ArrowSchemaSetTypeRunEndEncoded(schema->children[0]->dictionary, + NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaSetType(schema->children[0]->dictionary->children[1], + NANOARROW_TYPE_FLOAT), + NANOARROW_OK); + + ASSERT_EQ(ArrowArrayInitFromSchema(array, schema, &error), NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); + struct ArrowArray* indices = array->children[0]; + struct ArrowArray* values = indices->dictionary; + ASSERT_EQ(ArrowArrayAppendInt(values->children[0], 1), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendDouble(values->children[1], 1.0), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendInt(values->children[0], extended ? 3 : 2), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendDouble(values->children[1], 2.0), NANOARROW_OK); + if (extended) { + ASSERT_EQ(ArrowArrayAppendInt(values->children[0], 4), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendDouble(values->children[1], 3.0), NANOARROW_OK); + } + values->length = extended ? 4 : 2; + + ASSERT_EQ(ArrowArrayAppendInt(indices, extended ? 3 : 1), NANOARROW_OK); + array->length = 1; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(array, nullptr), NANOARROW_OK); +} + static std::vector DecodeMessageTypes(const struct ArrowBuffer* buffer) { std::vector message_types; struct ArrowBufferView remaining; @@ -568,6 +614,45 @@ static std::vector DecodeDictionaryIds(const struct ArrowBuffer* buffer return dictionary_ids; } +static std::vector DecodeDictionaryDeltaFlags(const struct ArrowBuffer* buffer) { + std::vector is_delta; + struct ArrowBufferView remaining; + remaining.data.as_uint8 = buffer->data; + remaining.size_bytes = buffer->size_bytes; + struct ArrowIpcDecoder decoder; + struct ArrowError error; + ArrowIpcDecoderInit(&decoder); + + while (remaining.size_bytes > 0) { + int result = ArrowIpcDecoderVerifyHeader(&decoder, remaining, &error); + if (result == ENODATA) { + break; + } + + EXPECT_EQ(result, NANOARROW_OK) << error.message; + if (result != NANOARROW_OK) { + break; + } + + if (decoder.message_type == NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH) { + result = ArrowIpcDecoderDecodeHeader(&decoder, remaining, &error); + EXPECT_EQ(result, NANOARROW_OK) << error.message; + if (result != NANOARROW_OK) { + break; + } + is_delta.push_back(decoder.dictionary->is_delta); + } + + int64_t message_size = ((decoder.header_size_bytes + 7) / 8) * 8 + + ((decoder.body_size_bytes + 7) / 8) * 8; + remaining.data.as_uint8 += message_size; + remaining.size_bytes -= message_size; + } + + ArrowIpcDecoderReset(&decoder); + return is_delta; +} + static void MakeNestedDictionaryStructArray(struct ArrowArray* array, struct ArrowSchema* schema, const char* inner_value1 = "bar") { @@ -674,7 +759,10 @@ TEST(NanoarrowIpcWriter, ReemitsParentWhenNestedDictionaryChanges) { NANOARROW_OK) << error.message; - EXPECT_EQ(DecodeDictionaryIds(output.get()), (std::vector{1, 0, 1, 0})); + EXPECT_EQ(DecodeDictionaryIds(output.get()), + (std::vector{1, 0, 1, 0})); + EXPECT_EQ(DecodeDictionaryDeltaFlags(output.get()), + (std::vector{0, 0, 0, 0})); struct ArrowIpcInputStream input; ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); @@ -848,6 +936,168 @@ TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { "baz"); } +TEST(NanoarrowIpcWriter, EmitsAppendOnlyDictionaryAsDelta) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get(), "foo", "bar", "baz"); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + EXPECT_EQ(DecodeDictionaryDeltaFlags(output.get()), (std::vector{0, 1})); + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_input = std::make_shared( + arrow::Buffer::Wrap(output->data, output->size_bytes)); + auto maybe_arrow_reader = arrow::ipc::RecordBatchStreamReader::Open(arrow_input); + ASSERT_TRUE(maybe_arrow_reader.ok()) << maybe_arrow_reader.status(); + auto arrow_reader = maybe_arrow_reader.ValueUnsafe(); + std::shared_ptr arrow_batch1; + std::shared_ptr arrow_batch2; + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch1).ok()); + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch2).ok()); + auto arrow_dictionary2 = + std::static_pointer_cast(arrow_batch2->column(0)); + auto arrow_values2 = + std::static_pointer_cast(arrow_dictionary2->dictionary()); + ASSERT_EQ(arrow_values2->length(), 3); + EXPECT_EQ(arrow_values2->GetString(2), "baz"); +#endif + + struct ArrowIpcInputStream input; + ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); + nanoarrow::UniqueArrayStream reader; + ASSERT_EQ(ArrowIpcArrayStreamReaderInit(reader.get(), &input, nullptr), NANOARROW_OK); + nanoarrow::UniqueSchema roundtrip_schema; + ASSERT_EQ(ArrowArrayStreamGetSchema(reader.get(), roundtrip_schema.get(), &error), + NANOARROW_OK) + << error.message; + nanoarrow::UniqueArray roundtrip1; + nanoarrow::UniqueArray roundtrip2; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip1.get(), &error), NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip2.get(), &error), NANOARROW_OK) + << error.message; + ASSERT_EQ(roundtrip1->children[0]->dictionary->length, 2); + ASSERT_EQ(roundtrip2->children[0]->dictionary->length, 3); + + nanoarrow::UniqueArrayView roundtrip_view2; + ASSERT_EQ( + ArrowArrayViewInitFromSchema(roundtrip_view2.get(), roundtrip_schema.get(), &error), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayViewSetArray(roundtrip_view2.get(), roundtrip2.get(), &error), + NANOARROW_OK); + EXPECT_EQ(ArrowArrayViewGetStringUnsafe(roundtrip_view2->children[0]->dictionary, 2), + ArrowCharView("baz")); +} + +TEST(NanoarrowIpcWriter, EmitsAppendOnlyRunEndDictionaryAsDelta) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeRunEndDictionaryStructArray(array1.get(), schema.get(), false); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeRunEndDictionaryStructArray(array2.get(), unused_schema.get(), true); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + EXPECT_EQ(DecodeDictionaryDeltaFlags(output.get()), (std::vector{0, 1})); + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_input = std::make_shared( + arrow::Buffer::Wrap(output->data, output->size_bytes)); + auto maybe_arrow_reader = arrow::ipc::RecordBatchStreamReader::Open(arrow_input); + ASSERT_TRUE(maybe_arrow_reader.ok()) << maybe_arrow_reader.status(); + auto arrow_reader = maybe_arrow_reader.ValueUnsafe(); + std::shared_ptr arrow_batch1; + std::shared_ptr arrow_batch2; + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch1).ok()); + ASSERT_TRUE(arrow_reader->ReadNext(&arrow_batch2).ok()); + auto arrow_dictionary2 = + std::static_pointer_cast(arrow_batch2->column(0)); + ASSERT_EQ(arrow_dictionary2->dictionary()->length(), 4); +#endif +} + +TEST(NanoarrowIpcWriter, WritesAppendOnlyDictionaryToFile) { + struct ArrowError error; + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get()); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get(), "foo", "bar", "baz"); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) + << error.message; + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + auto* private_data = static_cast(writer->private_data); + EXPECT_EQ(private_data->footer.dictionary_blocks.size_bytes, + 2 * sizeof(struct ArrowIpcFileBlock)); + ASSERT_EQ(ArrowIpcWriterFinalizeFile(writer.get(), &error), NANOARROW_OK) + << error.message; + +#if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) + auto arrow_input = std::make_shared( + arrow::Buffer::Wrap(output->data, output->size_bytes)); + auto maybe_arrow_reader = arrow::ipc::RecordBatchFileReader::Open(arrow_input); + ASSERT_TRUE(maybe_arrow_reader.ok()) << maybe_arrow_reader.status(); + auto arrow_reader = maybe_arrow_reader.ValueUnsafe(); + ASSERT_EQ(arrow_reader->num_record_batches(), 2); + auto maybe_batch2 = arrow_reader->ReadRecordBatch(1); + ASSERT_TRUE(maybe_batch2.ok()) << maybe_batch2.status(); + auto arrow_dictionary2 = std::static_pointer_cast( + maybe_batch2.ValueUnsafe()->column(0)); + auto arrow_values2 = + std::static_pointer_cast(arrow_dictionary2->dictionary()); + ASSERT_EQ(arrow_values2->length(), 3); + EXPECT_EQ(arrow_values2->GetString(2), "baz"); +#endif +} + TEST(NanoarrowIpcWriter, RejectsChangedDictionaryInFile) { struct ArrowError error; nanoarrow::UniqueSchema schema; diff --git a/src/nanoarrow/nanoarrow_ipc.h b/src/nanoarrow/nanoarrow_ipc.h index 40fff14f6..c712e35bf 100644 --- a/src/nanoarrow/nanoarrow_ipc.h +++ b/src/nanoarrow/nanoarrow_ipc.h @@ -1191,6 +1191,10 @@ NANOARROW_DLL ArrowErrorCode ArrowIpcWriterWriteDictionaryBatch( /// \brief Write an entire stream (including EOS) to the output byte stream /// +/// Identical dictionaries are suppressed. When a dictionary grows by appending values, +/// the writer emits the appended values as a dictionary delta; other changes are emitted +/// as replacements in stream mode and return EINVAL in file mode. +/// /// Errors are propagated from the underlying encoder, array stream, and output byte /// stream. NANOARROW_DLL ArrowErrorCode ArrowIpcWriterWriteArrayStream(struct ArrowIpcWriter* writer, From 2dfe12c8c84ef1d63e360179311ba9339354b1c7 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 2 Sep 2026 01:02:54 -0400 Subject: [PATCH 13/16] fix(ipc): use dictionary delta flag type --- src/nanoarrow/ipc/writer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 593beb4fb..042369129 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -585,7 +585,7 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionaryBatchIfChanged( struct ArrowIpcWriterDictionaryCacheEntry* cached = ArrowIpcWriterFindDictionaryCacheEntry(private, dictionary_id); int values_equal = 0; - int is_delta = 0; + char is_delta = 0; int cached_was_added = 0; const struct ArrowArray* values_to_encode = NULL; if (cached != NULL) { From 6f249a751c7592b4620b24eebad28d563e6428fe Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 2 Sep 2026 09:33:25 -0400 Subject: [PATCH 14/16] fix(ipc): make dictionary comparisons valgrind-clean --- src/nanoarrow/ipc/writer.c | 30 +++++++++++++++++++ src/nanoarrow/ipc/writer_test.cc | 51 ++++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index 042369129..de51fe07c 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -485,6 +485,30 @@ static ArrowErrorCode ArrowIpcWriterArrayViewInitLike(struct ArrowArrayView* out return NANOARROW_OK; } +static void ArrowIpcWriterCanonicalizeBitmapPadding( + struct ArrowArray* array, const struct ArrowArrayView* array_view) { + int64_t remainder = array->length % 8; + if (remainder != 0) { + uint8_t mask = (uint8_t)((1U << remainder) - 1U); + for (int i = 0; i < NANOARROW_MAX_FIXED_BUFFERS; i++) { + if (array_view->layout.element_size_bits[i] == 1) { + struct ArrowBuffer* buffer = ArrowArrayBuffer(array, i); + if (buffer->size_bytes > 0) { + buffer->data[buffer->size_bytes - 1] &= mask; + } + } + } + } + + for (int64_t i = 0; i < array->n_children; i++) { + ArrowIpcWriterCanonicalizeBitmapPadding(array->children[i], array_view->children[i]); + } + + if (array->dictionary != NULL) { + ArrowIpcWriterCanonicalizeBitmapPadding(array->dictionary, array_view->dictionary); + } +} + static ArrowErrorCode ArrowIpcWriterMaterializeArrayView(const struct ArrowArrayView* src, int64_t offset, int64_t length, struct ArrowArray* out, @@ -516,6 +540,12 @@ static ArrowErrorCode ArrowIpcWriterMaterializeArrayView(const struct ArrowArray if (result == NANOARROW_OK) { result = ArrowArrayFinishBuildingDefault(out, error); } + if (result == NANOARROW_OK) { + // Arrow bitmaps do not require producers to initialize padding bits. Clear them so + // physical comparisons of two otherwise identical materialized arrays never read + // indeterminate data and do not treat padding as part of dictionary identity. + ArrowIpcWriterCanonicalizeBitmapPadding(out, src); + } if (result != NANOARROW_OK && out->release != NULL) { ArrowArrayRelease(out); diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index e912faf14..3a881b98b 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -471,7 +471,8 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, struct ArrowSchema* schema, const char* value0 = "foo", const char* value1 = "bar", - const char* value2 = nullptr) { + const char* value2 = nullptr, + bool value1_is_null = false) { ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); ASSERT_EQ(ArrowSchemaInitFromType(schema->children[0], NANOARROW_TYPE_INT32), @@ -488,7 +489,11 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, ASSERT_EQ(ArrowArrayStartAppending(array), NANOARROW_OK); ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value0)), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value1)), NANOARROW_OK); + if (value1_is_null) { + ASSERT_EQ(ArrowArrayAppendNull(values, 1), NANOARROW_OK); + } else { + ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value1)), NANOARROW_OK); + } if (value2 != nullptr) { ASSERT_EQ(ArrowArrayAppendString(values, ArrowCharView(value2)), NANOARROW_OK); } @@ -832,6 +837,41 @@ TEST(NanoarrowIpcWriter, DoesNotRepeatUnchangedDictionary) { NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH})); } +TEST(NanoarrowIpcWriter, DoesNotRepeatUnchangedNullableDictionary) { + struct ArrowError error; + + nanoarrow::UniqueSchema schema; + nanoarrow::UniqueArray array1; + MakeDictionaryStructArray(array1.get(), schema.get(), "foo", "unused", nullptr, + /*value1_is_null=*/true); + + nanoarrow::UniqueSchema unused_schema; + nanoarrow::UniqueArray array2; + MakeDictionaryStructArray(array2.get(), unused_schema.get(), "foo", "unused", nullptr, + /*value1_is_null=*/true); + + nanoarrow::UniqueArrayStream array_stream; + ASSERT_EQ(ArrowBasicArrayStreamInit(array_stream.get(), schema.get(), 2), NANOARROW_OK); + ArrowBasicArrayStreamSetArray(array_stream.get(), 0, array1.get()); + ArrowBasicArrayStreamSetArray(array_stream.get(), 1, array2.get()); + + nanoarrow::UniqueBuffer output; + nanoarrow::ipc::UniqueOutputStream out_stream; + ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); + + nanoarrow::ipc::UniqueWriter writer; + ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); + ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), + NANOARROW_OK) + << error.message; + + EXPECT_EQ(DecodeMessageTypes(output.get()), + (std::vector{NANOARROW_IPC_MESSAGE_TYPE_SCHEMA, + NANOARROW_IPC_MESSAGE_TYPE_DICTIONARY_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH, + NANOARROW_IPC_MESSAGE_TYPE_RECORD_BATCH})); +} + TEST(NanoarrowIpcWriter, EmitsChangedDictionary) { struct ArrowError error; @@ -1228,11 +1268,10 @@ TEST(NanoarrowIpcWriter, RoundtripDictionaryStream) { EXPECT_EQ(std::string(v0.data, v0.size_bytes), "foo"); EXPECT_EQ(std::string(v1.data, v1.size_bytes), "bar"); - roundtrip_array.reset(); - ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip_array.get(), &error), - NANOARROW_OK) + nanoarrow::UniqueArray end; + ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), end.get(), &error), NANOARROW_OK) << error.message; - EXPECT_EQ(roundtrip_array->release, nullptr); + EXPECT_EQ(end->release, nullptr); } // A struct array with a single int32 column of repeating values (i.e., compressible) From b86d802f610dc6ffc085013b5238b0c04062b99b Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Wed, 2 Sep 2026 09:43:27 -0400 Subject: [PATCH 15/16] test(ipc): cover dictionary validation errors --- src/nanoarrow/ipc/decoder_test.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/nanoarrow/ipc/decoder_test.cc b/src/nanoarrow/ipc/decoder_test.cc index f7f290002..590052204 100644 --- a/src/nanoarrow/ipc/decoder_test.cc +++ b/src/nanoarrow/ipc/decoder_test.cc @@ -876,6 +876,14 @@ TEST(NanoarrowIpcTest, NanoarrowIpcDecodeDictionaryBatch) { data.data.as_uint8 += decoder.header_size_bytes; data.size_bytes -= decoder.header_size_bytes; + // The convenience APIs without a dictionary memo must reject dictionary-encoded + // fields instead of returning an array with an unresolved dictionary. + struct ArrowArrayView* unresolved_view; + ASSERT_EQ(ArrowIpcDecoderDecodeArrayView(&decoder, data, 0, &unresolved_view, &error), + ENOTSUP); + EXPECT_STREQ(error.message, + "Can't decode a dictionary-encoded field without ArrowIpcDictionaries"); + // Decode the entire batch and check the dictionary struct ArrowArrayView* batch_view; ASSERT_EQ(ArrowIpcDecoderDecodeArrayViewWithDictionaries( From 6b8b6c7b874957055a18e8659823340f0baad561 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Thu, 10 Sep 2026 15:32:28 -0400 Subject: [PATCH 16/16] fix(ipc): adapt dictionary deltas to array view append --- src/nanoarrow/ipc/decoder_test.cc | 18 +++++++++---- src/nanoarrow/ipc/encoder_test.cc | 2 ++ src/nanoarrow/ipc/writer.c | 4 +-- src/nanoarrow/ipc/writer_test.cc | 45 +++++++------------------------ 4 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/nanoarrow/ipc/decoder_test.cc b/src/nanoarrow/ipc/decoder_test.cc index 590052204..280401bae 100644 --- a/src/nanoarrow/ipc/decoder_test.cc +++ b/src/nanoarrow/ipc/decoder_test.cc @@ -2319,7 +2319,8 @@ static std::shared_ptr MakeDeltaDictionaryIndices(bool extended) { static void AssertReadsArrowCppDeltaStream( const std::shared_ptr& dictionary_array1, - const std::shared_ptr& dictionary_array2) { + const std::shared_ptr& dictionary_array2, + const char* expected_error = nullptr) { auto schema = arrow::schema({arrow::field("dictionary", dictionary_array1->type())}); auto expected1 = arrow::RecordBatch::Make(schema, 4, {dictionary_array1}); auto expected2 = arrow::RecordBatch::Make(schema, 4, {dictionary_array2}); @@ -2360,8 +2361,13 @@ static void AssertReadsArrowCppDeltaStream( nanoarrow::UniqueArray roundtrip2; ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip1.get(), &error), NANOARROW_OK) << error.message; - ASSERT_EQ(ArrowArrayStreamGetNext(reader.get(), roundtrip2.get(), &error), NANOARROW_OK) - << error.message; + int result = ArrowArrayStreamGetNext(reader.get(), roundtrip2.get(), &error); + if (expected_error != nullptr) { + EXPECT_EQ(result, ENOTSUP); + EXPECT_STREQ(error.message, expected_error); + return; + } + ASSERT_EQ(result, NANOARROW_OK) << error.message; auto maybe_roundtrip1 = arrow::ImportRecordBatch(roundtrip1.get(), arrow_schema); auto maybe_roundtrip2 = arrow::ImportRecordBatch(roundtrip2.get(), arrow_schema); ASSERT_TRUE(maybe_roundtrip1.ok()) << maybe_roundtrip1.status(); @@ -2384,7 +2390,7 @@ TEST_P(DeltaDictionaryTypeTest, ReadsArrowCppDeltaStream) { AssertReadsArrowCppDeltaStream(maybe_array1.ValueUnsafe(), maybe_array2.ValueUnsafe()); } -TEST(NanoarrowIpcTest, ReadsArrowCppDenseUnionDictionaryDelta) { +TEST(NanoarrowIpcTest, RejectsArrowCppDenseUnionDictionaryDelta) { arrow::Int8Builder type_ids_builder; arrow::Int32Builder offsets_builder; arrow::Int32Builder ints_builder; @@ -2416,7 +2422,9 @@ TEST(NanoarrowIpcTest, ReadsArrowCppDenseUnionDictionaryDelta) { dictionary_type, MakeDeltaDictionaryIndices(true), values2); ASSERT_TRUE(maybe_array1.ok()) << maybe_array1.status(); ASSERT_TRUE(maybe_array2.ok()) << maybe_array2.status(); - AssertReadsArrowCppDeltaStream(maybe_array1.ValueUnsafe(), maybe_array2.ValueUnsafe()); + AssertReadsArrowCppDeltaStream( + maybe_array1.ValueUnsafe(), maybe_array2.ValueUnsafe(), + "Appending array views is not supported for dense_union"); } INSTANTIATE_TEST_SUITE_P(NanoarrowIpcDecoder, DeltaDictionaryTypeTest, diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index 267e872d1..2bb1a42bf 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -1676,6 +1676,8 @@ TEST(NanoarrowIpcTest, NanoarrowIpcEncoderCompressorWithoutOutput) { EIO); EXPECT_THAT(error.message, ::testing::StartsWith("Compressor produced no output for a buffer of")); +} + TEST(NanoarrowIpcTest, NanoarrowIpcEncoderRejectsNestedDictionaryBatch) { nanoarrow::UniqueSchema schema; ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_INT32), NANOARROW_OK); diff --git a/src/nanoarrow/ipc/writer.c b/src/nanoarrow/ipc/writer.c index de51fe07c..c6df51c1f 100644 --- a/src/nanoarrow/ipc/writer.c +++ b/src/nanoarrow/ipc/writer.c @@ -789,8 +789,8 @@ static ArrowErrorCode ArrowIpcWriterWriteDictionariesForArrayView( for (int64_t i = n_dictionaries - 1; i >= 0; i--) { int emitted = 0; result = ArrowIpcWriterWriteDictionaryBatchIfChanged( - writer, dictionary_views[i].dictionary_id, - dictionary_views[i].values_view, dictionary_views[i].force_emit, + writer, dictionary_views[i].dictionary_id, dictionary_views[i].values_view, + dictionary_views[i].force_emit, /*allow_delta=*/1, &emitted, error); if (result != NANOARROW_OK) { break; diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc index 3a881b98b..3b1666609 100644 --- a/src/nanoarrow/ipc/writer_test.cc +++ b/src/nanoarrow/ipc/writer_test.cc @@ -282,29 +282,6 @@ TEST(NanoarrowIpcWriter, WriteDictionaryBatch) { // one block tracked in file mode EXPECT_EQ(p2->footer.dictionary_blocks.size_bytes, sizeof(struct ArrowIpcFileBlock)); - - int64_t bytes_written = p2->bytes_written; - EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch(writer2.get(), /*dictionary_id=*/0, - /*is_delta=*/0, values_view.get(), &error), - ENOTSUP); - EXPECT_STREQ(error.message, - "IPC file writing supports exactly one non-delta dictionary batch"); - EXPECT_EQ(p2->bytes_written, bytes_written); - EXPECT_EQ(p2->footer.dictionary_blocks.size_bytes, sizeof(struct ArrowIpcFileBlock)); - - nanoarrow::ipc::UniqueOutputStream stream3; - nanoarrow::UniqueBuffer output3; - ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(stream3.get(), output3.get()), NANOARROW_OK); - - nanoarrow::ipc::UniqueWriter writer3; - ASSERT_EQ(ArrowIpcWriterInit(writer3.get(), stream3.get()), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterStartFile(writer3.get(), &error), NANOARROW_OK) - << error.message; - EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch(writer3.get(), /*dictionary_id=*/0, - /*is_delta=*/1, values_view.get(), &error), - ENOTSUP); - EXPECT_STREQ(error.message, - "IPC file writing supports exactly one non-delta dictionary batch"); } TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { @@ -440,10 +417,10 @@ TEST(NanoarrowIpcWriter, RoundtripDeltaDictionaryStream) { ASSERT_EQ(ArrowIpcWriterWriteDictionaryBatch(file_writer.get(), 0, /*is_delta=*/1, delta_values_view.get(), &error), NANOARROW_OK); - EXPECT_EQ(ArrowIpcWriterWriteDictionaryBatch( - file_writer.get(), 0, /*is_delta=*/0, - batch2_view->children[0]->dictionary, &error), - EINVAL); + EXPECT_EQ( + ArrowIpcWriterWriteDictionaryBatch(file_writer.get(), 0, /*is_delta=*/0, + batch2_view->children[0]->dictionary, &error), + EINVAL); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), batch2_view.get(), &error), NANOARROW_OK); ASSERT_EQ(ArrowIpcWriterWriteArrayView(file_writer.get(), nullptr, &error), @@ -507,8 +484,7 @@ static void MakeDictionaryStructArray(struct ArrowArray* array, } static void MakeRunEndDictionaryStructArray(struct ArrowArray* array, - struct ArrowSchema* schema, - bool extended) { + struct ArrowSchema* schema, bool extended) { struct ArrowError error; ASSERT_EQ(ArrowSchemaInitFromType(schema, NANOARROW_TYPE_STRUCT), NANOARROW_OK); ASSERT_EQ(ArrowSchemaAllocateChildren(schema, 1), NANOARROW_OK); @@ -518,7 +494,7 @@ static void MakeRunEndDictionaryStructArray(struct ArrowArray* array, ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[0]), NANOARROW_OK); ArrowSchemaInit(schema->children[0]->dictionary); ASSERT_EQ(ArrowSchemaSetTypeRunEndEncoded(schema->children[0]->dictionary, - NANOARROW_TYPE_INT32), + NANOARROW_TYPE_INT32), NANOARROW_OK); ASSERT_EQ(ArrowSchemaSetType(schema->children[0]->dictionary->children[1], NANOARROW_TYPE_FLOAT), @@ -764,10 +740,8 @@ TEST(NanoarrowIpcWriter, ReemitsParentWhenNestedDictionaryChanges) { NANOARROW_OK) << error.message; - EXPECT_EQ(DecodeDictionaryIds(output.get()), - (std::vector{1, 0, 1, 0})); - EXPECT_EQ(DecodeDictionaryDeltaFlags(output.get()), - (std::vector{0, 0, 0, 0})); + EXPECT_EQ(DecodeDictionaryIds(output.get()), (std::vector{1, 0, 1, 0})); + EXPECT_EQ(DecodeDictionaryDeltaFlags(output.get()), (std::vector{0, 0, 0, 0})); struct ArrowIpcInputStream input; ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, output.get()), NANOARROW_OK); @@ -1109,8 +1083,7 @@ TEST(NanoarrowIpcWriter, WritesAppendOnlyDictionaryToFile) { ASSERT_EQ(ArrowIpcOutputStreamInitBuffer(out_stream.get(), output.get()), NANOARROW_OK); nanoarrow::ipc::UniqueWriter writer; ASSERT_EQ(ArrowIpcWriterInit(writer.get(), out_stream.get()), NANOARROW_OK); - ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) - << error.message; + ASSERT_EQ(ArrowIpcWriterStartFile(writer.get(), &error), NANOARROW_OK) << error.message; ASSERT_EQ(ArrowIpcWriterWriteArrayStream(writer.get(), array_stream.get(), &error), NANOARROW_OK) << error.message;