Skip to content

perf: prune oversized dictionaries before IPC spilling - #27015

Merged
alamb merged 1 commit into
apache:mainfrom
polarsignals:prune-spill-dictionaries
Nov 4, 2026
Merged

alamb merged 1 commit into
apache:mainfrom
polarsignals:prune-spill-dictionaries

Conversation

@brancz

@brancz brancz commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Compact dictionary value tables larger than their key arrays before traversing retained physical children and writing IPC spill files.

Which issue does this PR close?

What changes are included in this PR?

Compaction of dicts if there are more values than indices and tests.

What is the testing strategy for this PR?

Unit tests

Are there any user-facing changes?

No user facing changes, just an improvement to reduce I/O and total size written to disks when spilling.

@alamb

Compact dictionary value tables larger than their key arrays before traversing retained physical children and writing IPC spill files.

Exercise nested dictionaries, repeated and null keys, and uncompressed, LZ4 and Zstd spill round trips through SpillManager. Document that physical-child traversal does not guarantee complete ListView reachability pruning.

Amp-Thread-ID: https://ampcode.com/threads/T-01a192af-b64d-77b7-a93b-d443169ddf9f
Co-authored-by: Amp <[email protected]>
@brancz

brancz commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

cc @Jefffrey

#[case(SpillCompression::Lz4Frame)]
#[case(SpillCompression::Zstd)]
#[tokio::test]
async fn test_spill_prunes_unused_nested_dictionary_values(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this test makes sense to me, looking at some of the other tests here I don't think it fits in any of them.

@alamb
alamb added this pull request to the merge queue Nov 4, 2026
@alamb

alamb commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Thank you @brancz and @AdamGS

Merged via the queue into apache:main with commit 6a98807 Nov 4, 2026
45 of 46 checks passed
alamb added a commit that referenced this pull request Nov 6, 2026
…ackport #27015) (#27107)

## Which issue does this PR close?

- Backport of #27015 to
`branch-56`
- Part of #24461
- Closes #27014

Co-authored-by: Frederic Branczyk <[email protected]>
Co-authored-by: Amp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prune unused dictionary values before spilling to Arrow IPC

3 participants