Skip to content

bench: scalar functions over dictionary-encoded columns - #24586

Open
radmirnovii wants to merge 1 commit into
apache:mainfrom
radmirnovii:dict-peel-bench
Open

bench: scalar functions over dictionary-encoded columns#24586
radmirnovii wants to merge 1 commit into
apache:mainfrom
radmirnovii:dict-peel-bench

Conversation

@radmirnovii

Copy link
Copy Markdown

Which issue does this PR close?

Refs #20935 and #19458.

Rationale for this change

Scalar functions over dictionary-encoded columns have no benchmark coverage:
neither the hand-written dictionary arms (e.g. reverse's, from #23930) nor
the cast away from the encoding that every function without preservation pays
(e.g. encode). This lands the baseline first, so follow-up work generalizing
the arms shows its delta on stable benchmark ids.

What changes are included in this PR?

One criterion benchmark:

  • reverse over a dictionary of its own per batch (cold) and one shared
    across batches the way a Parquet column chunk delivers them (warm) — today
    the two cost the same, since an arm cannot reuse anything across batches;
  • cast_away: the dictionary cast to its value type and the function called
    once per row — what any function without encoding preservation pays;
  • flat: the same rows with no encoding anywhere, as a bound.

encode cannot be called over a dictionary today, so it has no
dictionary-typed groups yet.

Are these changes tested?

cargo bench -p datafusion-physical-expr --bench scalar_function_dictionary
runs clean.

Are there any user-facing changes?

No.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NysGeXTG5opiJKBApsAe5H

@neilconway neilconway left a comment

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.

Scalar functions over dictionary-encoded columns have no benchmark coverage:
neither the hand-written dictionary arms (e.g. reverse's, from #23930)

#23930 added benchmarks for reverse over dictionary-encoded columns, which are partially duplicated by this PR, albeit in a different crate (datafusion-functions vs datafusion-physical-expr). Can you take a look at the existing benchmarks and consolidate them together with your additions?

// group would quietly measure a warm dictionary under a cold name.
for distinct in [8usize, 256, 512, ROWS]
.into_iter()
.filter(|_| *dictionary_calls)

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.

Seems a bit odd to write the conditional this way, since *dictionary_calls is invariant over the inner loop. Write if *dictionary_calls { ... } instead? Here and below.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

@github-actions github-actions Bot added the functions Changes to functions implementation label Aug 24, 2026
@radmirnovii

Copy link
Copy Markdown
Author

Consolidated into physical-expr/benches/dictionary_encoding.rs, the datafusion-functions copy removed — that direction because physical-expr already dev-depends on functions, not the reverse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants