Skip to content

feat(ipc): add dictionary batch write support - #928

Open
rustyconover wants to merge 11 commits into
apache:mainfrom
Query-farm:fix/dictionary-batch-message-metadata
Open

feat(ipc): add dictionary batch write support#928
rustyconover wants to merge 11 commits into
apache:mainfrom
Query-farm:fix/dictionary-batch-message-metadata

Conversation

@rustyconover

@rustyconover rustyconover commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add DictionaryBatch metadata/body encoding and low-level writer APIs
  • teach the stream writer to discover dictionaries, emit them dependency-first, and suppress unchanged values
  • advertise dictionary replacement support and emit changed dictionaries as replacements in streams
  • reject changed dictionaries when writing IPC files, while retaining explicit low-level delta-batch writing
  • expand native, Arrow C++, file, nested-dictionary, replacement, metadata, and failure-path coverage

This builds on and is intended to supersede #926. Its original commit is retained as the first commit here, preserving authorship and credit to @niekverw.

The generic array-view appender has been extracted to #930. Dictionary-delta decoding is prepared on a separate branch and will be submitted after #930 merges so its diff contains only decoder work. Automatic append-only delta emission is intentionally no longer part of this PR.

IPC behavior

Streams emit full DictionaryBatch replacements when dictionary values change and advertise DICTIONARY_REPLACEMENT in the schema. Files reject dictionary changes through the high-level array-stream writer. The low-level ArrowIpcWriterWriteDictionaryBatch() API can still encode explicitly requested delta batches and records their blocks in footer order.

Validation

  • native CMake build passed
  • native CTest suite: 343/343 passed (4 optional codec tests skipped)
  • the pre-split branch passed the full upstream CI matrix, including Arrow C++ integration, namespaced/bundled builds, sanitizers, and Valgrind

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.89189% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.40%. Comparing base (f55f85b) to head (0951095).

Files with missing lines Patch % Lines
src/nanoarrow/ipc/encoder.c 54.09% 0 Missing and 28 partials ⚠️
src/nanoarrow/ipc/writer.c 80.64% 5 Missing and 19 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #928      +/-   ##
==========================================
+ Coverage   79.30%   79.40%   +0.10%     
==========================================
  Files         106      106              
  Lines       16223    16397     +174     
  Branches     1882     1942      +60     
==========================================
+ Hits        12865    13020     +155     
+ Misses       2177     2151      -26     
- Partials     1181     1226      +45     

☔ 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.

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

@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 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

Copy link
Copy Markdown
Contributor Author

Yep I'll break them out!

Rusty

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

Copy link
Copy Markdown
Contributor Author

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.

@rustyconover
rustyconover force-pushed the fix/dictionary-batch-message-metadata branch from 8db5007 to 0951095 Compare September 2, 2026 18:13
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.

4 participants