feat(ipc): Don't re-emit unchanged dictionaries when writing streams; fix nested dictionary and file writing by rustyconover · Pull Request #928 · apache/arrow-nanoarrow

rustyconover · 2026-09-02T05:01:47Z

This PR improves dictionary writing in the IPC writer/encoder. Per review, it has been split: delta dictionary decoding is now #955, and the writer no longer computes or emits delta dictionaries (applications can still write deltas explicitly with ArrowIpcWriterWriteDictionaryBatch(..., is_delta = 1, ...)).

ArrowIpcWriterWriteArrayStream() no longer writes every dictionary before every batch. A dictionary is skipped when it has the same length/offset/null count and points to the same buffers (recursively, including nested dictionaries) as the previous batch's dictionary — e.g., when the producer shares one dictionary across batches, as the nanoarrow IPC reader does. Comparison is by address rather than by value. To make that safe (a released buffer's address can't be reused for different values while it is being compared), the writer keeps the previous batch alive until the next one is written. ArrowIpcWriterWriteDictionaryBatch() only sees a borrowed view and always writes.
Adds ArrowIpcEncoderSetDictionaryReplacement(); the writer declares DICTIONARY_REPLACEMENT in streams, and the reader no longer rejects streams that declare it.
Encodes the children of dictionary value fields (e.g., struct dictionaries with dictionary-encoded children) and writes nested dictionaries before their parent.
Attaches ArrowIpcEncoderSetMessageMetadata() metadata to DictionaryBatch messages.
File mode: the footer schema now resolves dictionary IDs and one DictionaryBatch is allowed per ID (a repeated ID returns EINVAL; deltas still return ENOTSUP). Unchanged dictionaries from WriteArrayStream() are skipped.
Enables the write roundtrip for the dictionary, nested dictionary, shared dictionary and extension integration files.

Tests cover a shared dictionary being written once (stream and file), changing a nested dictionary re-emitting its parent, and a producer that recycles a released dictionary's memory for different values. Locally (aarch64 Linux, Debug, ZSTD/LZ4), ctest passes with and without Arrow C++ 21.0.0, and the writer/encoder/reader/files tests are clean under valgrind.

🤖 Generated with Claude Code

codecov-commenter · 2026-09-02T05:07:47Z

Codecov Report

❌ Patch coverage is 75.00000% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.33%. Comparing base (e5f3d0a) to head (08b9807).

Files with missing lines	Patch %	Lines
src/nanoarrow/ipc/writer.c	75.00%	3 Missing and 11 partials ⚠️
src/nanoarrow/ipc/encoder.c	75.00%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #928      +/-   ##
==========================================
+ Coverage   78.24%   78.33%   +0.09%     
==========================================
  Files         106      106              
  Lines       16901    16950      +49     
  Branches     2012     2033      +21     
==========================================
+ Hits        13224    13278      +54     
+ Misses       2435     2425      -10     
- Partials     1242     1247       +5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paleolimbot

Thank you for this!

I will take a closer look in the next few days, but can I ask that the delta dictionary and/or appender piece be separated from the IPC write piece? All three are independently useful and I think it will be a smoother process if they're PRed separately as well.

rustyconover · 2026-09-02T17:09:19Z

Yep I'll break them out!

Rusty

rustyconover · 2026-09-02T18:13:12Z

Split completed as requested:

feat: append array views to arrays #930 contains the generic ArrowArrayAppendArrayView() functionality as a one-commit core-array PR.
This PR has been rebuilt on current main and now contains only dictionary IPC encoding/writing (no decoder changes, appender, quadratic-delta helper, or automatic delta emission).
The dictionary-delta decoder is preserved on Query-farm:feat/ipc-dictionary-delta-decoding; I will submit it after feat: append array views to arrays #930 merges so its upstream diff contains only decoder work.

The original full branch is preserved as Query-farm:split/pr928-full.

## Summary - add `ArrowArrayAppendArrayView()` as a core array-building API - append primitive, nested, union, list-view, and run-end encoded values from an `ArrowArrayView` - preserve source slicing semantics and report unsupported sliced run-end encoded inputs - add direct native coverage for primitive, nested/sliced, union, and run-end encoded arrays ## Context This extracts the generic array-view appender from #928 as requested in review. It is independently useful and will be the prerequisite for a separate dictionary-delta decoding PR; #928 no longer contains the appender or decoder changes. ## Validation - native CMake build passed - native CTest suite: 334/334 passed (4 optional codec tests skipped)

paleolimbot

Thank you for this!

I think this PR contains 3 orthogonal changes:

Decoding delta dictionaries + some small fixes to the decoder
Avoiding dictionary replacement when emitting dictionaries that are identical + assorted fixes to the encoder for nested dictionaries
Encoding/writing delta dictionaries

I did a first pass on everything and I think we can get all these merged soon!

paleolimbot · 2026-09-18T03:27:07Z

+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]);
+  }
+}


I don't think we support views yet for IPC decoding at all yet? #824

Removed. The delta decoding moved to #955, and there is no view-specific handling there.

paleolimbot · 2026-09-18T03:35:51Z

+    ArrowArrayMove(&dictionary->current_value, &combined);
+
+    ArrowIpcArrayPrepareForAppend(&combined, array_view);
+    ArrowErrorCode result = ArrowArrayReserve(&combined, value->length);
+    if (result == NANOARROW_OK) {
+      result = ArrowArrayAppendStorageFromArrayView(&combined, array_view, error);
+    }
+    if (result == NANOARROW_OK) {
+      result = ArrowIpcArraySetDictionaries(&combined, value);
+    }


For the PR that does contain this, a good pattern is to define a function that can fail and doesn't own any input pointers:

ArrowErrorCode ArrowIpcDoAppend(struct ArrowArray* combined, struct ArrowArrayView array_view, struct ArrowError* error) { NANOARROW_RETURN_NOT_OK(...); return NANOARROW_OK; }

then here you can just check once and release the temporary

ArrowErrorCode result = ArrowIpcDoAppend(...); if (result != NANOARROW_OK) { // release stuff return result; }

Done in #955: ArrowIpcDictionaryConcatenate() can fail and doesn't own its inputs, and the caller checks once and releases the temporary.

paleolimbot · 2026-09-18T03:39:25Z

+enum class DeltaDictionaryValueCase {
+  kBoolean,
+  kInt64,
+  kInt64WithNull,
+  kUInt64,
+  kDouble,
+  kString,
+  kBinary,
+  kDecimal128,
+  kList,
+  kStruct,
+  kFixedSizeList
+};


Can you reuse enum ArrowType for this one?

Done in #955: the test parameters are now enum ArrowType values.

paleolimbot · 2026-09-18T03:41:22Z

+TEST(NanoarrowIpcTest, RejectsArrowCppDenseUnionDictionaryDelta) {
+  arrow::Int8Builder type_ids_builder;
+  arrow::Int32Builder offsets_builder;


I think we can skip this test unless there's something special about rejecting a type in the decoder implementation

Removed in #955.

paleolimbot · 2026-09-18T04:04:11Z

+  // 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);


The usual streaming loop should be using ArrowIpcDecoderDecodeArrayViewWithDictionaries(), which in a perfect world never was shared (however, it probably had a shared backing buffer on its first decode because most decoding happens from a shared buffer).

Rather than trying to unshare, can we check whether this array can be appended to (and if possibly delay the sharing of it until it is requested as an array that is not just a view?). Totally ok if not possible, but if so, we should punt on the trying to unshare piece and implement that in a follow up.

Punted. #955 drops the unshare helper (and the common/utils.c change) and always builds a new combined array, so each delta copies the dictionary. Appending in place when the dictionary isn't shared can come in a follow-up.

paleolimbot · 2026-09-18T04:15:57Z

+static void ArrowIpcWriterCanonicalizeBitmapPadding(
+    struct ArrowArray* array, const struct ArrowArrayView* array_view) {
+  int64_t remainder = array->length % 8;


It would help to put the comment about canonicalizing the padding helping to minimize reemitting a dictionary here.

Other parts of nanoarrow typically avoid this by zeroing out the last byte of a bitmap when reserving but it is sometimes hard to guarantee that everywhere.

This code is gone. With comparison by buffer address there is no need to canonicalize bitmap padding.

paleolimbot · 2026-09-18T04:16:44Z

+  ArrowErrorCode result = ArrowArrayInitFromArrayView(out, src, error);
+  if (result == NANOARROW_OK) {
+    result = ArrowArrayStartAppending(out);
+  }


The same nested function strategy for avoiding these repeated result checks applies here, too

Gone along with the materialized copies; there are no more result chains in the writer.

paleolimbot · 2026-09-18T04:21:10Z

+static ArrowErrorCode ArrowIpcWriterCompareMaterializedArrays(
+    const struct ArrowArray* lhs, const struct ArrowArray* rhs,
+    const struct ArrowArrayView* shape, int* out, struct ArrowError* error) {


Rather than compare by value (possibly slower than just writing another dictionary to the stream), probably comparing the buffer pointers makes more sense. If it's a shared array it should be pointing to the same data.

Done: dictionaries are now compared by shape and buffer addresses, recursively. To keep that safe, WriteArrayStream() holds the previous batch until the next one is written, so a released buffer's address can't be reused for different values mid-comparison. There's a test with a producer that recycles memory this way. WriteDictionaryBatch() only sees a borrowed view and always writes.

paleolimbot · 2026-09-18T04:22:40Z

+      if (result != NANOARROW_OK) {
+        goto cleanup;
+      }
+      is_delta = 1;


You can use the nested function approach for shared cleanup (we don't use goto in nanoarrow, or at least we haven't yet)

No more goto. The only shared cleanup is in the existing outer WriteArrayStream() function.

paleolimbot · 2026-09-18T04:25:30Z

+  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;
+    }


I don't think that the writer should emit delta dictionaries in this way...this is a kind of calculation that application code should do, which may have access to things like C++ that can do it more efficiently.

In any case, emitting delta dictionaries is a third scope here for a separate PR

Agreed. Delta emission is removed from the writer entirely. Applications that want deltas can write them with ArrowIpcWriterWriteDictionaryBatch(..., is_delta = 1, ...), so I'm not planning a separate PR for it.

ArrowIpcEncoderSetMessageMetadata() was ignored by ArrowIpcEncoderEncodeSimpleDictionaryBatch(); encode it like the schema and record batch messages do. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The IPC Field for a dictionary-encoded column describes the dictionary value type, so its children must come from schema->dictionary rather than from the index schema. This replaces the ENOTSUP for dictionary values with children (e.g., struct dictionaries whose children are themselves dictionary-encoded). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Add ArrowIpcEncoderSetDictionaryReplacement() to declare the DICTIONARY_REPLACEMENT feature in an encoded Schema message. The writer enables it in stream mode, where a dictionary ID may be written more than once, and the reader no longer rejects streams that declare it (the decoder already replaces dictionaries). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

On main, a file could contain exactly one DictionaryBatch, so a schema with more than one dictionary-encoded field couldn't be written, and the footer schema couldn't resolve its dictionary IDs when finalizing the file. Record the dictionary encodings of the footer schema and track the dictionary IDs written to the file: a second batch for the same ID returns EINVAL because IPC files don't support dictionary replacement (deltas remain unsupported when writing files). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

When dictionary values contain dictionary-encoded children, a reader needs the nested dictionaries before it can decode the DictionaryBatch that references them. Keep assigning IDs in schema order but write each dictionary after the dictionaries nested in its values. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ArrowIpcWriterWriteArrayStream() wrote every dictionary before every record batch. Skip a dictionary when it has the same length, offset, null count, and buffer addresses (recursively) as the corresponding dictionary of the previous array, which is the case when a producer shares one dictionary among the arrays of a stream (e.g., the nanoarrow IPC reader). Comparing addresses is only safe while both arrays are alive, so the writer keeps the previous array (if it has dictionaries) until the next one is written. ArrowIpcWriterWriteDictionaryBatch() only sees a borrowed view that it can't keep alive and always writes the dictionary. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Write the integration files back using ArrowIpcWriterWriteArrayStream(), which writes dictionaries (writing the arrays with ArrowIpcWriterWriteArrayView() alone does not), and enable the write roundtrip for the dictionary, nested dictionary, shared dictionary, and extension (whose dict-extension column is dictionary-encoded) files. Because the reader shares dictionaries among the batches it decodes, these roundtrips also exercise skipping unchanged dictionaries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

rustyconover · 2026-10-05T02:03:59Z

Thanks for the thorough review! I've split this as you suggested:

feat: Decode dictionary delta batches #955: decoding dictionary delta batches (decoder only).
This PR: not re-emitting unchanged dictionaries (now by buffer address), plus the encoder and writer fixes for nested dictionaries, DICTIONARY_REPLACEMENT, dictionary batch metadata and file mode. It is rebuilt from current main and contains no decoder or delta changes.
Delta emission in the writer is dropped, per your comment that this belongs in application code. The existing ArrowIpcWriterWriteDictionaryBatch(..., is_delta = 1, ...) covers that.

The two PRs are independent and can be reviewed and merged in either order. I've replied to each inline comment with where it was addressed.

paleolimbot

Thank you!

Two things probably worth fixing here, plus possibly worth a merge of main since there's one IPC-related fix there since this was last pushed.

paleolimbot · 2026-10-13T02:25:52Z

+  for (int64_t i = 0; i < lhs->n_buffers; i++) {
+    if (lhs->buffers[i] != rhs->buffers[i]) {


I think the only exception here is for StringView or BinaryView, where its final buffer is a buffer of sizes that I think is typically generated on each export and may not be the same.

(IPC for nanoarrow doesn't currently support this but someday it will and is probably worth fixing now)

Good catch. ArrowIpcWriterArraysShareBuffers() now gets the ArrowArrayView alongside the arrays (for the storage types) and skips the last buffer of a BinaryView/StringView array. It still compares the views and variadic data buffers, which hold the values. I noted this in the WriteArrayStream() docs too. I couldn't add a test that reaches it, because the encoder can't write view types yet.

paleolimbot · 2026-10-13T02:47:50Z

+        ArrowErrorSet(
+            error, "Arrow IPC files do not support replacement of dictionary ID %" PRId64,
+            dictionary_id);
+        return EINVAL;


Codex flagged for me here that ENOTSUP would be more consistent with the other things that can't be done in IPC file writing, although I don't personally mind either one

Switched to ENOTSUP, to match the delta case right above it, and updated the docs for WriteDictionaryBatch() and WriteArrayStream() and the two tests.

Split out of #928 as requested in review. This PR only adds decoding of dictionary delta batches; the writer changes remain in #928. Previously, `ArrowIpcDecoderDecodeDictionary()` failed with "Dictionary concatenation is not yet supported" when it got a DictionaryBatch with `isDelta` set and the current dictionary was non-empty. Now the delta values are appended: a new array is built from the current dictionary value and the delta using `ArrowArrayAppendStorageFromArrayView()` (#930), and it replaces the stored dictionary. Arrays decoded earlier keep referencing their previous dictionary value. Value types that `ArrowArrayAppendStorageFromArrayView()` can't append (e.g., unions) fail with `ENOTSUP`. Changes from the review of #928: - Removed the string/binary-view-specific handling (IPC doesn't support views yet, #824). - Replaced the `if (result == NANOARROW_OK)` chains with a helper that doesn't own its inputs, plus a single check and release. - Dropped the "try to unshare" optimization and the internal `common/utils.c` helper. Each delta copies the dictionary; appending in place when nothing else references the dictionary can come in a follow-up. - Tests use `enum ArrowType` for parameters, and the dense union rejection test is removed. This also fixes a double release on the error path of `ArrowIpcDecoderDecodeDictionaryInternal()`: `ArrowArrayMoveShared()` already releases its input on failure, so the temporary is now only released if it is still live. Tests: `NanoarrowIpcDecodeDictionaryBatch` covers deltas, including a clone held across a delta keeping its old value. With `NANOARROW_BUILD_TESTS_WITH_ARROW`, streams written by Arrow C++ with `emit_dictionary_deltas = true` are read back for bool, int64 (with nulls), uint64, double, string, binary, decimal128, list, struct and fixed-size list values. Locally (aarch64 Linux, Debug, ZSTD/LZ4), ctest passes with and without Arrow C++ 21.0.0, and the decoder tests are clean under valgrind. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… a repeated file dictionary ID Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

rustyconover · 2026-10-16T16:01:44Z

Thanks! Both comments are addressed, and I've merged main (which includes #960 and #955). ctest passes locally (397 tests), and the writer tests are clean under ASan/UBSan.

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 0e55dbe to de091e0 Compare September 2, 2026 14:08

paleolimbot reviewed Sep 2, 2026

View reviewed changes

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from de091e0 to 8db5007 Compare September 2, 2026 18:12

rustyconover mentioned this pull request Sep 2, 2026

feat: append array views to arrays #930

Merged

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 8db5007 to 0951095 Compare September 2, 2026 18:13

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 0951095 to 067826a Compare September 10, 2026 19:32

rustyconover changed the title ~~feat(ipc): add dictionary batch write support~~ feat(ipc): support dictionary updates and deltas Sep 10, 2026

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 067826a to 6b8b6c7 Compare September 10, 2026 20:34

paleolimbot reviewed Sep 18, 2026

View reviewed changes

rustyconover and others added 7 commits October 4, 2026 11:42

fix(ipc): attach message metadata to dictionary batches

a34ce84

ArrowIpcEncoderSetMessageMetadata() was ignored by ArrowIpcEncoderEncodeSimpleDictionaryBatch(); encode it like the schema and record batch messages do. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 6b8b6c7 to 3e4046a Compare October 5, 2026 02:03

rustyconover mentioned this pull request Oct 5, 2026

feat: Decode dictionary delta batches #955

Merged

rustyconover changed the title ~~feat(ipc): support dictionary updates and deltas~~ feat(ipc): Don't re-emit unchanged dictionaries when writing streams; fix nested dictionary and file writing Oct 5, 2026

paleolimbot approved these changes Oct 13, 2026

View reviewed changes

This was referenced Oct 16, 2026

feat(ipc): Add ArrowIpcWriterSetMessageMetadata #967

Open

feat(ipc): Add ArrowIpcWriterWriteArrayViewWithDictionaries #968

Open

rustyconover and others added 2 commits October 16, 2026 12:00

Don't compare view arrays' variadic sizes buffers; return ENOTSUP for…

2b00267

… a repeated file dictionary ID Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into split/dict-writer-dedup

08b9807

		for (int64_t i = 0; i < lhs->n_buffers; i++) {
		if (lhs->buffers[i] != rhs->buffers[i]) {

Conversation

rustyconover commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

rustyconover commented Sep 2, 2026

Uh oh!

rustyconover commented Sep 2, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustyconover commented Oct 5, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustyconover commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

rustyconover commented Sep 2, 2026 •

edited

Loading

codecov-commenter commented Sep 2, 2026 •

edited

Loading