Skip to content

refactor: separate compact IN-list pruning threshold from the default… - #25044

Open
goutamadwant wants to merge 1 commit into
apache:mainfrom
goutamadwant:refactor-compact-in-list-threshold-24710
Open

refactor: separate compact IN-list pruning threshold from the default…#25044
goutamadwant wants to merge 1 commit into
apache:mainfrom
goutamadwant:refactor-compact-in-list-threshold-24710

Conversation

@goutamadwant

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Small string lists are not uniformly faster with compact pruning. The crossover depends on the number of statistics containers and their distribution, so this retains the existing boundary rather than removing it.

The representation threshold currently reuses MAX_IN_LIST_SIZE, coupling it to the default maximum eligible list length. These are separate decisions.

What changes are included in this PR?

  • Introduce a private MIN_COMPACT_IN_LIST_SIZE of 21, preserving the current strictly-greater-than-20 boundary.
  • Keep the public default cap and zero/over-cap behavior unchanged.
  • Add boundary coverage for both IN and NOT IN.
  • Extend the existing benchmark with one-container cases and small-list homogeneous NOT IN cases. Make NULL-result assertions follow the selected representation rather than the default cap.

What is the testing strategy for this PR?

  • All 100 pruning unit tests pass, including the new cap/threshold coverage.
  • The final benchmark smoke passes all 292 assertion cases.
  • The extended workspace suite passes 11,264 Rust tests, with eight existing tests ignored, and all 511 SQL logic test files.
  • cargo clippy --all-targets --all-features -- -D warnings and the documented dev/rust_lint.sh checks, including strict documentation, pass.
  • Balanced release-nonlto baseline/forced-compact runs demonstrate the tradeoff. With four literals and 4,096 containers, IN evaluation changes from 36.5–36.8 µs to 98.4–99.1 µs; NOT IN changes from 28.9–29.3 µs to 85.7–86.7 µs. At 16 containers, compact IN is faster, about 1.6 µs versus 5.5–5.6 µs. Compact construction is also cheaper.
  • Homogeneous singleton containers favor the existing short-circuit path: with 20 literals and 4,096 containers, NOT IN changes from 13.3–13.6 µs to 121.7–122.9 µs when forced compact. Expanded-expression controls remained close across repeated runs.
  • Reproduce with cargo bench -p datafusion-pruning --bench string_in_list_pruning --profile release-nonlto. To compare the small compact form, temporarily set the private minimum to 1 and rerun; that experimental change is not included in this PR.

These are pruning microbenchmarks, not whole-query speedups or evidence of a universal optimal threshold.

Are there any user-facing changes?

No. The default cap, representation boundary, pruning behavior, and public APIs remain unchanged.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.72%. Comparing base (1b6dc92) to head (63a9e8e).

Files with missing lines Patch % Lines
datafusion/pruning/src/pruning_predicate.rs 96.15% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25044      +/-   ##
==========================================
- Coverage   81.72%   81.72%   -0.01%     
==========================================
  Files        1127     1127              
  Lines      416310   416334      +24     
  Branches   416310   416334      +24     
==========================================
+ Hits       340224   340233       +9     
- Misses      56095    56105      +10     
- Partials    19991    19996       +5     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Revisit the 20-value lower bound for compact IN-list pruning

2 participants