Skip to content

feat(ipc): Add ArrowIpcWriterSetMessageMetadata - #967

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

rustyconover wants to merge 1 commit 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

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.25%. Comparing base (e5f3d0a) to head (aafc9b0).

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.24%   78.25%   +0.01%     
==========================================
  Files         106      106              
  Lines       16901    16915      +14     
  Branches     2012     2015       +3     
==========================================
+ Hits        13224    13237      +13     
  Misses       2435     2435              
- Partials     1242     1243       +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.

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.

2 participants