Skip to content

feat(ipc): Add ArrowIpcWriterSetMessageMetadata - #967

Open
rustyconover wants to merge 3 commits into
apache:mainfrom
rustyconover:feat/ipc-writer-message-metadata
Open

rustyconover wants to merge 3 commits into
apache:mainfrom
rustyconover:feat/ipc-writer-message-metadata

Conversation

@rustyconover

Copy link
Copy Markdown
Contributor

ArrowIpcEncoderSetMessageMetadata() (#924, narrowed in #937) attaches Message::custom_metadata to the next message an encoder encodes. An ArrowIpcWriter keeps its encoder private, though, so a caller using the writer has no way to attach metadata to the RecordBatch messages it writes. The only option is to drop down to the encoder and redo the writer's framing, footer bookkeeping and dictionary handling by hand.

This PR adds ArrowIpcWriterSetMessageMetadata(writer, metadata, error). The writer holds the metadata and attaches it to the next RecordBatch message written by ArrowIpcWriterWriteArrayView() or ArrowIpcWriterWriteArrayStream(). The metadata applies to that one message. Passing NULL clears metadata that has been set but not yet written. Like the encoder function, it takes ownership of the buffer.

The metadata is held by the writer rather than set on the encoder right away. Setting it on the encoder would attach it to whichever message is encoded next, and that can be a DictionaryBatch written ahead of the batch. This happens both with an explicit ArrowIpcWriterWriteDictionaryBatch() call and inside ArrowIpcWriterWriteArrayStream(). RecordBatch-level metadata belongs on the RecordBatch, so the writer passes it to the encoder just before encoding that batch.

Things worth a look in review:

  • With WriteArrayStream() the metadata lands on the first RecordBatch only. Per-batch metadata over a stream would need a callback or a per-array side channel, which seemed out of scope. The batch-at-a-time pattern is SetMessageMetadata() + WriteArrayView().
  • feat(ipc): Don't re-emit unchanged dictionaries when writing streams; fix nested dictionary and file writing #928 makes encoder-level metadata apply to DictionaryBatch messages too. The two compose: this PR never sets encoder metadata until right before the RecordBatch is encoded. They touch neighbouring code in writer.c, so whichever lands second will need a small rebase. I can do that.

Motivation: I'm moving the IPC layer of a DuckDB extension from Arrow C++ to nanoarrow. Its RPC protocol carries per-batch state in RecordBatch custom_metadata (what Arrow C++ writes for RecordBatchWriter::WriteRecordBatch(batch, custom_metadata)), and this was the one thing the writer couldn't express.

Tests:

  • MessageMetadataAttachesToNextRecordBatch sets metadata, writes a DictionaryBatch and then a RecordBatch, and checks that only the RecordBatch carries it. It also checks that the following batch doesn't inherit the metadata, and that set-then-cleared metadata is never written.
  • MessageMetadataWithArrayStream covers WriteArrayStream() writing its own dictionaries.

ctest passes locally (macOS arm64, Debug, ZSTD/LZ4, 390 tests). The IPC writer, encoder and files tests are clean under ASan/UBSan.

🤖 Generated with Claude Code

Message::custom_metadata can be attached through the encoder
(ArrowIpcEncoderSetMessageMetadata), but an ArrowIpcWriter's encoder is
private, so a writer could not attach metadata to the RecordBatch messages
it writes. The new function holds the metadata in the writer and attaches
it to the next RecordBatch, not to DictionaryBatch messages written ahead
of it (by ArrowIpcWriterWriteDictionaryBatch() or within
ArrowIpcWriterWriteArrayStream()).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.45%. Comparing base (b9e0f5f) to head (6fe0f30).

Files with missing lines Patch % Lines
src/nanoarrow/ipc/writer.c 92.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
+ Coverage   78.44%   78.45%   +0.01%     
==========================================
  Files         106      106              
  Lines       17050    17064      +14     
  Branches     2018     2021       +3     
==========================================
+ Hits        13375    13388      +13     
  Misses       2432     2432              
- Partials     1243     1244       +1     

☔ 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 paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Codex kindly reminded me that if writing the batch returns an error code for some reason that the metadata will still be queued for the next batch. I'm not sure if that matters in practice, possibly just something to document so that the caller knows to clear the metadata if that matters.

paleolimbot pushed a commit that referenced this pull request Oct 20, 2026
`ArrowIpcWriterWriteArrayStream()` writes the DictionaryBatch messages
each array depends on, but only when it is given a whole
`ArrowArrayStream`. A caller that produces batches one at a time and
writes them with `ArrowIpcWriterWriteArrayView()` has no public way to
get those dictionaries written. It would have to work out the dictionary
IDs itself, matching the depth-first order the schema encoder assigns
them in, and call `ArrowIpcWriterWriteDictionaryBatch()` for each one.

This PR adds `ArrowIpcWriterWriteArrayViewWithDictionaries(writer, view,
error)`. It writes a DictionaryBatch for each dictionary-encoded field
of the view and then the RecordBatch. That is the same per-array step
`WriteArrayStream()` already performs, and `WriteArrayStream()` now
calls the new function, so there is one code path. The behavior is
unchanged: every dictionary is written in full before every RecordBatch,
and file mode has the same limits as `WriteArrayStream()`. The doc
comment says so rather than promising anything beyond that.

Things worth a look in review:

- #928 changes how `WriteArrayStream()` writes dictionaries. It skips a
dictionary that is unchanged from the previous batch, and writes nested
dictionaries before their parent. Because it does that by keeping the
previous `ArrowArray` alive and comparing buffer addresses, the skip
can't apply here: this function only gets a borrowed view. The other
#928 changes (dependency-first order, file-mode bookkeeping) would apply
here too, because the two paths share the helper. Whichever lands second
needs a small rebase in `writer.c` and in `MakeDictionaryStructArray`,
which I've extended with the same `value1` parameter #928 adds. I can do
that rebase.
- `in` must not be NULL (a DCHECK, matching
`ArrowIpcWriterWriteDictionaryBatch()`). EOS still goes through
`ArrowIpcWriterWriteArrayView(writer, NULL, ...)`.

Motivation: I'm moving the IPC layer of a DuckDB extension from Arrow
C++ to nanoarrow. It writes one batch at a time to a worker process, and
DuckDB `ENUM` columns arrive as dictionary arrays. This is the
batch-at-a-time version of what `WriteArrayStream()` already does.
Together with #967 (`ArrowIpcWriterSetMessageMetadata`), it covers what
the extension needed from Arrow C++'s
`RecordBatchWriter::WriteRecordBatch()`.

Tests:
- `WriteArrayViewWithDictionariesMatchesArrayStream` writes a
dictionary-encoded batch through the new function and through
`WriteArrayStream()`, and checks the bytes are identical.
- `WriteArrayViewWithDictionariesPerBatch` writes two batches with
different dictionaries one at a time. It checks the message sequence
(schema, dictionary, batch, dictionary, batch) and that the reader
resolves each batch against its own dictionary.

ctest passes locally (macOS arm64, Debug, ZSTD/LZ4, 390 tests). One run
hit a single failure in `NanoarrowIntegrationTestBatch`, which doesn't
touch the writer. It didn't recur in isolation or in five more full
runs. The IPC writer and reader tests are clean under ASan/UBSan.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rustyconover and others added 2 commits October 19, 2026 23:42
…ge-metadata

# Conflicts:
#	src/nanoarrow/ipc/writer_test.cc
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants