Skip to content

fix(ipc): Use the decoder's decompressor for dictionary batches - #994

Open
ump45nose wants to merge 1 commit into
apache:mainfrom
ump45nose:fix/936-dictionary-decompressor
Open

ump45nose wants to merge 1 commit into
apache:mainfrom
ump45nose:fix/936-dictionary-decompressor

Conversation

@ump45nose

Copy link
Copy Markdown
Contributor

Closes #936.

When a DictionaryBatch body is compressed, its values are decoded by the nested per-dictionary decoder, and that decoder always initialized its own default serial decompressor. So a decompressor set on the decoder with ArrowIpcDecoderSetDecompressor() was used for record batches but silently ignored for dictionary batches.

This passes the parent decoder's decompressor down to the nested decoder when decoding dictionary values: ArrowIpcDecoderDecodeArrayViewInternal() now takes an optional decompressor (NULL keeps the current behaviour of using the decoder's own), and ArrowIpcDecoderDecodeDictionaryInternal() passes the parent's (initializing the default one if none was set). No public API changes.

The other point in the issue, dictionary sub-decoders not inheriting the parent's metadata version, was already handled in #960 (dictionary->decoder.metadata_version = decoder->metadata_version), so this only covers the decompressor.

Testing

  • Added NanoarrowIpcEncoderCompressedDictionaryBatchCustomDecompressor, which reuses the compressed dictionary batch test with a counting decompressor (wrapping ArrowIpcSerialDecompressor()) set on the decoder and checks it was called. It fails on main (0 calls) and passes with this change.
  • Debug build with NANOARROW_IPC=ON NANOARROW_BUILD_TESTS=ON NANOARROW_IPC_WITH_ZSTD=ON NANOARROW_IPC_WITH_LZ4=ON: ctest 412/412 passed.
  • clang-format 16.0.6 (the pre-commit version) applied.

This PR was prepared with AI assistance; I reviewed the changes and ran the checks above.

Compressed DictionaryBatch values were decompressed by the nested
dictionary decoder's default serial decompressor, so a decompressor set
with ArrowIpcDecoderSetDecompressor() was ignored for dictionaries. Pass
the parent decoder's decompressor down when decoding the dictionary
values.

Closes apache#936.

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.

Nested decoders do not propagate some types of required parameters into dictionary sub-decoders

1 participant