Skip to content

fix: inverted null_percent logic in in_list benchmark - #19204

Merged
adriangb merged 1 commit into
apache:mainfrom
geoffreyclaude:fix/in_list_benchmark
Dec 8, 2025
Merged

fix: inverted null_percent logic in in_list benchmark#19204
adriangb merged 1 commit into
apache:mainfrom
geoffreyclaude:fix/in_list_benchmark

Conversation

@geoffreyclaude

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

N/A - benchmark fix discovered during performance analysis of #18832.

Rationale for this change

The in_list benchmark (introduced in #4068) had inverted null generation logic: null_percent=0 was producing 100% nulls instead of 0% nulls.

What changes are included in this PR?

Fix the random_bool(null_percent).then(...) pattern to use random_bool(1.0 - null_percent) so that null_percent correctly represents the percentage of null values.

Are these changes tested?

Benchmark-only change. Verified by running the benchmark and observing expected performance characteristics.

Are there any user-facing changes?

No.

@adriangb
adriangbforce-pushed the fix/in_list_benchmark branch from c5e8898 to e1e4da3CompareDecember 8, 2025 17:39
@adriangb
adriangb added this pull request to the merge queueDec 8, 2025
Merged via the queue into apache:main with commit 662a3baDec 8, 2025
14 checks passed
github-merge-queueBot pushed a commit that referenced this pull request Dec 8, 2025
## Which issue does this PR close?
N/A - benchmark improvement
Stacked on top of #19204
## Rationale for this change
We need to measure InList performance on both StringArray (Utf8) and
StringViewArray (Utf8View) to compare Arrow's string representations.
## What changes are included in this PR?
Add Utf8View benchmarks for InList, refactored with generics to make
adding new array types trivial.
## Are these changes tested?
Benchmark-only change.
## Are there any user-facing changes?
No.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-exprChanges to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@geoffreyclaude@adriangb