Skip to content

Perf: Short circuit expensive comparissions in GroupColumn #23344

Open
Rich-T-kid wants to merge 3 commits into
apache:mainfrom
Rich-T-kid:rich-T-kid/short-circuit-groupColumn
Open

Perf: Short circuit expensive comparissions in GroupColumn #23344
Rich-T-kid wants to merge 3 commits into
apache:mainfrom
Rich-T-kid:rich-T-kid/short-circuit-groupColumn

Conversation

@Rich-T-kid

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

see #23342

Note : the exact ranking here is a bit arbitrary & i'm happy to change it. The core idea is that its much cheaper to do boolean checks and fixed size byte comparisons as opposed to chasing pointers to comparing strings.
an alternative approach may be to split this into two tiers:

  1. ints/bools/floats/
  2. bytes data

What changes are included in this PR?

  • Add compare_tier() to rank each supported DataType by comparison cost (1 = cheap fixed-width integers → 5 = expensive variable-length bytes)
  • Add compare_order: Vec to GroupValuesColumn, built once at construction via build_group_compare_order(), so vectorized_equal_to evaluates columns cheapest-first and maximizes early-exit savings before reaching expensive string comparisons

Are these changes tested?

This PR only change the order in which comparisons occurs. functionally nothing changes.

Are there any user-facing changes?

no

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 6, 2026
@Rich-T-kid Rich-T-kid changed the title initial draft Short circuit expensive comparissions in GroupColumn Jul 6, 2026
@Rich-T-kid

Rich-T-kid commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

cc @zhuqi-lucas as this is related to #22715
Could a maintainer run the benchmarks?

/// Used to order columns in [`GroupValuesColumn::compare_order`] so cheap comparisons
/// eliminate rows before expensive ones are evaluated.
/// see <https://github.com/apache/datafusion/issues/23342>
fn compare_tier(data_type: &DataType) -> u8 {

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.

Shouldn't this belong to a common place as this can be used across other operators like HashJoins

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suppose 🤔 , is there a specific case where you think this would be useful for hashJoins?

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.

When JoinKeyComparator finds groups matching groups per row similarly, correct me if I am wrong. Here the compare method can also use this optimized ordering right ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Im honestly not too familiar with the join operator logic. but presumably the same concepts should apply there. Ideally i'd like to move forward with #23342. but I can make a follow up ticket to move this into a higher modules for code re-use

@Rich-T-kid Rich-T-kid changed the title Short circuit expensive comparissions in GroupColumn Perf: Short circuit expensive comparissions in GroupColumn Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Short circuit expensive comparissions in GroupColumn trait

2 participants