test: skip count distinct spill memory test under forced hash collisions - #24918
Merged
rluvaton merged 1 commit intoSep 3, 2026
Conversation
rluvaton
force-pushed
the
fix-count-distinct-spill-test-under-forced-hash-collisions
branch
2 times, most recently
from
September 3, 2026 07:29
19a64a6 to
fdd9d32
Compare
With force_hash_collisions every key hashes alike, so the hash repartitioning sends all groups to a single final stage whose table cannot fit the memory limit however well memory is released. The limit is sized for the real distribution across four final stages.
rluvaton
force-pushed
the
fix-count-distinct-spill-test-under-forced-hash-collisions
branch
from
September 3, 2026 07:30
fdd9d32 to
e484973
Compare
rluvaton
marked this pull request as draft
September 3, 2026 07:38
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24918 +/- ##
=======================================
Coverage 81.63% 81.63%
=======================================
Files 1123 1123
Lines 409963 409963
Branches 409963 409963
=======================================
+ Hits 334685 334687 +2
+ Misses 55605 55598 -7
- Partials 19673 19678 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rluvaton
marked this pull request as ready for review
September 3, 2026 09:28
Weijun-H
approved these changes
Sep 3, 2026
Weijun-H
left a comment
Member
There was a problem hiding this comment.
LGTM
Non-blocking wording nit: filter_pushdown.rs adapts expectations under the feature rather than skipping the test, so consider removing that comparison from the PR description.
rluvaton
deleted the
fix-count-distinct-spill-test-under-forced-hash-collisions
branch
September 3, 2026 10:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Follow-up to #24888, which broke the
cargo test hash collisionsCI job on main.Rationale for this change
The memory limit test added in #24888 fails when built with
force_hash_collisions. Every key hashes to the same value there, so the hash repartition sends all 64 groups to one final stage. That single table needs 5.3 MB against the test's 4 MB pool, and it has nothing reserved yet, so there is nothing to spill. It fails no matter how well the accumulator releases memory, which is what the test is actually about.I tried a few ways to keep it running under the feature first:
They all hit the same thing: under forced collisions the total state and a single batch are the same size, and the pool would have to sit above one and below the other.
What changes are included in this PR?
The test and its helpers move into a module gated on
not(feature = "force_hash_collisions").What is the testing strategy for this PR?
cargo test -p datafusion --features force_hash_collisions --test core_integration count_distinct_releasesruns 0 tests. Without the feature it still runs and passes.Are there any user-facing changes?
No.
🤖 Generated with Claude Code