Skip to content

adapters: fix OOM in ad-hoc query hashing for large data#6016

Merged
swanandx merged 1 commit intomainfrom
query-hash-fix
Apr 14, 2026
Merged

adapters: fix OOM in ad-hoc query hashing for large data#6016
swanandx merged 1 commit intomainfrom
query-hash-fix

Conversation

@swanandx
Copy link
Copy Markdown
Contributor

Replace arrow-digest's RecordDigestV0 (order-dependent, required sorting the entire DataFrame) with a streaming, order-independent BatchHasher.

The old approach sorted rows by every column before hashing. For large views it lead to OOM. The new approach uses DataFusion's create_hashes with two independent ahash seeds, accumulating row hashes via wrapping addition (commutative — no sort needed).

Memory usage is O(batch_size).

Describe Manual Test Plan

tested by adding tests 💡

Checklist

  • Unit tests added/updated

Breaking Changes?

  • Others (specify)

Describe Incompatible Changes

Yes, it changes the HASH generation, invalidating older hashes for same data. But we only use it internally so should be fine.

Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

One blocker: please squash the [ci] apply automatic fixes commit. Dirty history is still a hard no for ready PRs.

fn new() -> Self {
Self {
rs1: ahash::RandomState::with_seeds('M' as u64, 'U' as u64, 'A' as u64, 'Y' as u64),
rs2: ahash::RandomState::with_seeds('T' as u64, 'H' as u64, 'A' as u64, 'I' as u64),
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.

lol

@swanandx swanandx force-pushed the query-hash-fix branch 2 times, most recently from 4aaaacf to e12ebf1 Compare April 13, 2026 08:47
@swanandx swanandx requested a review from mythical-fred April 13, 2026 08:48
Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

LGTM

@swanandx swanandx added this pull request to the merge queue Apr 14, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 14, 2026
Replace arrow-digest's RecordDigestV0 (order-dependent, required sorting
the entire DataFrame) with a streaming, order-independent BatchHasher.

The old approach sorted rows by every column before hashing. For
large views it lead to OOM. The new approach uses DataFusion's
create_hashes with two independent ahash seeds, accumulating row hashes
via wrapping addition (commutative — no sort needed).

Memory usage is O(batch_size).

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
@swanandx swanandx enabled auto-merge April 14, 2026 06:44
@swanandx swanandx added this pull request to the merge queue Apr 14, 2026
Merged via the queue into main with commit dc44a5f Apr 14, 2026
1 check passed
@swanandx swanandx deleted the query-hash-fix branch April 14, 2026 08:13
@Karakatiza666
Copy link
Copy Markdown
Contributor

@swanandx Does this hash rows, or record log (inserts + deletes)?

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