Skip to content

Commit d167450

Browse files
authored
fix: Correct join cardinality estimation for semi and anti joins with disjoint column ranges (#22674)
## Which issue does this PR close? - Closes #22673 ## Rationale for this change `estimate_join_cardinality` for semi-joins checks if ANY of the columns in the two join inputs are disjoint (comparing columns positionally); if so, it claims the join will not return any rows. This is wrong, for two reasons: 1. If two columns don't participate in the join key, they have no impact on the cardinality of the join result 2. Comparing arbitrary columns positionally is not a sensible thing to do in the first place A similar issue exists for anti-joins, except we assume the anti-join will return the entire join input in this case. We should instead just check for disjoint ranges between the pairs of columns that make up the join key. ## What changes are included in this PR? * Fix `estimate_join_cardinality` behavior in the face of disjoint column ranges that aren't join key columns * Refactor `estimate_join_cardinality`, rename a variable for clarity * Add unit test ## Are these changes tested? Yes, new test added. ## Are there any user-facing changes? Better plans / avoid buggy cardinality estimate.
1 parent bb121a8 commit d167450

1 file changed

Lines changed: 85 additions & 30 deletions

File tree

  • datafusion/physical-plan/src/joins

datafusion/physical-plan/src/joins/utils.rs

Lines changed: 85 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ fn estimate_join_cardinality(
475475
right_stats: Statistics,
476476
on: &JoinOn,
477477
) -> Option<PartialJoinStatistics> {
478-
let (left_col_stats, right_col_stats) = on
478+
let (left_key_stats, right_key_stats) = on
479479
.iter()
480480
.map(|(left, right)| {
481481
match (
@@ -500,12 +500,12 @@ fn estimate_join_cardinality(
500500
Statistics {
501501
num_rows: left_stats.num_rows,
502502
total_byte_size: Precision::Absent,
503-
column_statistics: left_col_stats,
503+
column_statistics: left_key_stats,
504504
},
505505
Statistics {
506506
num_rows: right_stats.num_rows,
507507
total_byte_size: Precision::Absent,
508-
column_statistics: right_col_stats,
508+
column_statistics: right_key_stats,
509509
},
510510
)?;
511511

@@ -545,38 +545,49 @@ fn estimate_join_cardinality(
545545
let is_left = matches!(join_type, JoinType::LeftSemi | JoinType::LeftAnti);
546546
let is_anti = matches!(join_type, JoinType::LeftAnti | JoinType::RightAnti);
547547

548-
let ((outer_stats, inner_stats), (outer_col_stats, inner_col_stats)) =
549-
if is_left {
550-
(
551-
(&left_stats, &right_stats),
552-
(&left_col_stats, &right_col_stats),
553-
)
554-
} else {
555-
(
556-
(&right_stats, &left_stats),
557-
(&right_col_stats, &left_col_stats),
558-
)
559-
};
548+
let (outer_stats, inner_stats, outer_key_stats, inner_key_stats) = if is_left
549+
{
550+
(&left_stats, &right_stats, &left_key_stats, &right_key_stats)
551+
} else {
552+
(&right_stats, &left_stats, &right_key_stats, &left_key_stats)
553+
};
560554

561555
let outer_rows = *outer_stats.num_rows.get_value()?;
562556

563-
let cardinality =
564-
if estimate_disjoint_inputs(outer_stats, inner_stats).is_some() {
565-
// Disjoint inputs: semi produces 0, anti keeps all rows.
566-
if is_anti { outer_rows } else { 0 }
557+
let outer_join_key_stats = Statistics {
558+
num_rows: outer_stats.num_rows,
559+
total_byte_size: Precision::Absent,
560+
column_statistics: outer_key_stats.clone(),
561+
};
562+
let inner_join_key_stats = Statistics {
563+
num_rows: inner_stats.num_rows,
564+
total_byte_size: Precision::Absent,
565+
column_statistics: inner_key_stats.clone(),
566+
};
567+
568+
let semi_cardinality =
569+
if estimate_disjoint_inputs(&outer_join_key_stats, &inner_join_key_stats)
570+
.is_some()
571+
{
572+
// If join keys are disjoint, no rows will match
573+
Some(0)
567574
} else {
568-
match estimate_semi_join_cardinality(
575+
estimate_semi_join_cardinality(
569576
&outer_stats.num_rows,
570577
&inner_stats.num_rows,
571-
outer_col_stats,
572-
inner_col_stats,
573-
) {
574-
Some(semi) if is_anti => outer_rows.saturating_sub(semi),
575-
Some(semi) => semi,
576-
None => outer_rows,
577-
}
578+
outer_key_stats,
579+
inner_key_stats,
580+
)
578581
};
579582

583+
// Semi joins keep the matching rows; anti joins keep the rest. When no
584+
// estimate is available, conservatively assume all outer rows pass.
585+
let cardinality = match (semi_cardinality, is_anti) {
586+
(Some(semi), true) => outer_rows.saturating_sub(semi),
587+
(Some(semi), false) => semi,
588+
(None, _) => outer_rows,
589+
};
590+
580591
let outer_stats = if is_left { left_stats } else { right_stats };
581592
Some(PartialJoinStatistics {
582593
num_rows: cardinality,
@@ -759,8 +770,8 @@ fn estimate_disjoint_inputs(
759770
fn estimate_semi_join_cardinality(
760771
outer_num_rows: &Precision<usize>,
761772
inner_num_rows: &Precision<usize>,
762-
outer_col_stats: &[ColumnStatistics],
763-
inner_col_stats: &[ColumnStatistics],
773+
outer_key_stats: &[ColumnStatistics],
774+
inner_key_stats: &[ColumnStatistics],
764775
) -> Option<usize> {
765776
let outer_rows = *outer_num_rows.get_value()?;
766777
if outer_rows == 0 {
@@ -774,7 +785,7 @@ fn estimate_semi_join_cardinality(
774785
let mut selectivity = 1.0_f64;
775786
let mut has_selectivity_estimate = false;
776787

777-
for (outer_stat, inner_stat) in outer_col_stats.iter().zip(inner_col_stats.iter()) {
788+
for (outer_stat, inner_stat) in outer_key_stats.iter().zip(inner_key_stats.iter()) {
778789
let outer_has_stats = outer_stat.distinct_count.get_value().is_some()
779790
|| (outer_stat.min_value.get_value().is_some()
780791
&& outer_stat.max_value.get_value().is_some());
@@ -3246,6 +3257,50 @@ mod tests {
32463257
Ok(())
32473258
}
32483259

3260+
#[test]
3261+
fn test_semi_anti_join_disjoint_check_uses_only_join_keys() {
3262+
let join_on = vec![(
3263+
Arc::new(Column::new("l_key", 0)) as _,
3264+
Arc::new(Column::new("r_key", 0)) as _,
3265+
)];
3266+
3267+
// Ranges for the join key overlap; ranges for the other column are disjoint
3268+
let left_stats = Statistics {
3269+
num_rows: Inexact(50),
3270+
total_byte_size: Absent,
3271+
column_statistics: vec![
3272+
create_column_stats(Inexact(1), Inexact(10), Absent, Absent),
3273+
create_column_stats(Inexact(100), Inexact(200), Absent, Absent),
3274+
],
3275+
};
3276+
let right_stats = Statistics {
3277+
num_rows: Inexact(10),
3278+
total_byte_size: Absent,
3279+
column_statistics: vec![
3280+
create_column_stats(Inexact(1), Inexact(10), Absent, Absent),
3281+
create_column_stats(Inexact(1000), Inexact(2000), Absent, Absent),
3282+
],
3283+
};
3284+
3285+
let left_semi = estimate_join_cardinality(
3286+
&JoinType::LeftSemi,
3287+
left_stats.clone(),
3288+
right_stats.clone(),
3289+
&join_on,
3290+
)
3291+
.map(|c| c.num_rows);
3292+
assert_eq!(left_semi, Some(50));
3293+
3294+
let left_anti = estimate_join_cardinality(
3295+
&JoinType::LeftAnti,
3296+
left_stats,
3297+
right_stats,
3298+
&join_on,
3299+
)
3300+
.map(|c| c.num_rows);
3301+
assert_eq!(left_anti, Some(0));
3302+
}
3303+
32493304
#[test]
32503305
fn test_calculate_join_output_ordering() -> Result<()> {
32513306
let left_ordering = LexOrdering::new(vec![

0 commit comments

Comments
 (0)