Repository navigation
chore: Add end-to-end benchmark for array_agg, code cleanup - #20496
Merged
comphead merged 1 commit intoFeb 23, 2026
Merged
Conversation
comphead
reviewed
Feb 23, 2026
| }) | ||
| }); | ||
|
|
||
| c.bench_function("array_agg_query_group_by_few_groups", |b| { |
Contributor
There was a problem hiding this comment.
please help me to understand what are narrow, mid, wide groups? is this how data distributed, the amount of groups?
Contributor
Author
There was a problem hiding this comment.
In the query, it is the # of groups; of course for such a simple query the # of groups follows directly from the distribution of the data. wide, mid, narrow are respectively:
// Integers randomly selected from a wide range of values, i.e. [0,
// u64::MAX], such that there are ~no repeated values.
Field::new("u64_wide", DataType::UInt64, false),
// Integers randomly selected from a mid-range of values [0, 1000),
// providing ~1000 distinct groups.
Field::new("u64_mid", DataType::UInt64, false),
// Integers randomly selected from a narrow range of values such that
// there are a few distinct values, but they are repeated often.
Field::new("u64_narrow", DataType::UInt64, false),wide and narrow existed already, I just wanted a workload somewhere in the middle when profiling array_agg().
comphead
approved these changes
Feb 23, 2026
comphead
left a comment
comphead
left a comment
Contributor
There was a problem hiding this comment.
Thanks @neilconway it is LGTM
de-bgunter
pushed a commit
to de-bgunter/datafusion
that referenced
this pull request
Mar 24, 2026
…0496) ## Which issue does this PR close? - Prep work for apache#20465 ## Rationale for this change - Add three queries to measure the end-to-end performance of `array_agg()`, as prep work for optimizing its performance. ## What changes are included in this PR? This PR also cleans up the `data_utils` benchmark code: - Seed the RNG once and use it for all data generation. The previous coding seeded an RNG but only used it for some data, and also used the same seed for every batch, which lead to repeated data (... I assume this was not the intent?) - The previous code made `u64_wide` a nullable field, but passed `9.0` for the `value_density` when generating data, which meant that no NULL values would ever be generated. Switch to making `u64_wide` non-nullable. - Fix up comments, remove a clippy suppress, various other cleanups. ## Are these changes tested? Yes. ## Are there any user-facing changes? No.
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?
array_aggis very slow #20465Rationale for this change
array_agg(), as prep work for optimizing its performance.What changes are included in this PR?
This PR also cleans up the
data_utilsbenchmark code:u64_widea nullable field, but passed9.0for thevalue_densitywhen generating data, which meant that no NULL values would ever be generated. Switch to makingu64_widenon-nullable.Are these changes tested?
Yes.
Are there any user-facing changes?
No.