From fe773bfeb95a5081238c8c6aedad2a0532c54405 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Fri, 3 Jan 2025 15:04:47 -0500 Subject: [PATCH 1/2] save some MRE::typ() calls --- src/transform/src/semijoin_idempotence.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/transform/src/semijoin_idempotence.rs b/src/transform/src/semijoin_idempotence.rs index c33f88add9635..e335c75191047 100644 --- a/src/transform/src/semijoin_idempotence.rs +++ b/src/transform/src/semijoin_idempotence.rs @@ -198,6 +198,9 @@ fn attempt_join_simplification( let input_mapper = JoinInputMapper::new(inputs); if let Some((ltr, rtl)) = semijoin_bijection(inputs, equivalences) { + // If semijoin_bijection returns `Some(...)`, then `inputs.len() == 2`. + assert_eq!(inputs.len(), 2); + // Collect the `Get` identifiers each input might present as. let ids0 = as_filtered_get(&inputs[0], gets_behind_gets) .iter() @@ -208,6 +211,10 @@ fn attempt_join_simplification( .map(|(id, _)| *id) .collect::>(); + // Record the types of the inputs, for use in both loops below. + let typ0 = inputs[0].typ().column_types; + let typ1 = inputs[1].typ().column_types; + // Consider replacing the second input for the benefit of the first. if distinct_on_keys_of(&inputs[1], &rtl) && input_mapper.input_arity(1) == equivalences.len() @@ -222,8 +229,6 @@ fn attempt_join_simplification( // The pushdown is for the benefit of CSE on the `A` expressions, // in the not uncommon case of nullable foreign keys in outer joins. // TODO: Discover the transform that would not require this code. - let typ0 = inputs[0].typ().column_types; - let typ1 = inputs[1].typ().column_types; let mut is_not_nulls = Vec::new(); for (col0, col1) in ltr.iter() { if !typ1[*col1].nullable && typ0[*col0].nullable { @@ -256,8 +261,6 @@ fn attempt_join_simplification( // The pushdown is for the benefit of CSE on the `A` expressions, // in the not uncommon case of nullable foreign keys in outer joins. // TODO: Discover the transform that would not require this code. - let typ0 = inputs[0].typ().column_types; - let typ1 = inputs[1].typ().column_types; let mut is_not_nulls = Vec::new(); for (col1, col0) in rtl.iter() { if !typ0[*col0].nullable && typ1[*col1].nullable { From 0a3a1a0cbca34efa0dfccb973e36d7755b8d5b88 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Fri, 3 Jan 2025 15:14:51 -0500 Subject: [PATCH 2/2] push through more savings on MRE::typ() --- src/transform/src/semijoin_idempotence.rs | 32 +++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/transform/src/semijoin_idempotence.rs b/src/transform/src/semijoin_idempotence.rs index e335c75191047..2d1a0237427bb 100644 --- a/src/transform/src/semijoin_idempotence.rs +++ b/src/transform/src/semijoin_idempotence.rs @@ -26,6 +26,7 @@ //! which we will transfer to the columns of `D` thereby forming `C`. use itertools::Itertools; +use mz_repr::RelationType; use std::collections::BTreeMap; use mz_expr::{Id, JoinInputMapper, LocalId, MirRelationExpr, MirScalarExpr, RECURSION_LIMIT}; @@ -212,13 +213,11 @@ fn attempt_join_simplification( .collect::>(); // Record the types of the inputs, for use in both loops below. - let typ0 = inputs[0].typ().column_types; - let typ1 = inputs[1].typ().column_types; + let typ0 = inputs[0].typ(); + let typ1 = inputs[1].typ(); // Consider replacing the second input for the benefit of the first. - if distinct_on_keys_of(&inputs[1], &rtl) - && input_mapper.input_arity(1) == equivalences.len() - { + if distinct_on_keys_of(&typ1, &rtl) && input_mapper.input_arity(1) == equivalences.len() { for mut candidate in list_replacements(&inputs[1], let_replacements, gets_behind_gets) { if ids0.contains(&candidate.id) { if let Some(permutation) = validate_replacement(<r, &mut candidate) { @@ -231,7 +230,9 @@ fn attempt_join_simplification( // TODO: Discover the transform that would not require this code. let mut is_not_nulls = Vec::new(); for (col0, col1) in ltr.iter() { - if !typ1[*col1].nullable && typ0[*col0].nullable { + if !typ1.column_types[*col1].nullable + && typ0.column_types[*col0].nullable + { is_not_nulls.push(MirScalarExpr::Column(*col0).call_is_null().not()) } } @@ -248,9 +249,7 @@ fn attempt_join_simplification( } } // Consider replacing the first input for the benefit of the second. - if distinct_on_keys_of(&inputs[0], <r) - && input_mapper.input_arity(0) == equivalences.len() - { + if distinct_on_keys_of(&typ0, <r) && input_mapper.input_arity(0) == equivalences.len() { for mut candidate in list_replacements(&inputs[0], let_replacements, gets_behind_gets) { if ids1.contains(&candidate.id) { if let Some(permutation) = validate_replacement(&rtl, &mut candidate) { @@ -263,7 +262,9 @@ fn attempt_join_simplification( // TODO: Discover the transform that would not require this code. let mut is_not_nulls = Vec::new(); for (col1, col0) in rtl.iter() { - if !typ0[*col0].nullable && typ1[*col1].nullable { + if !typ0.column_types[*col0].nullable + && typ1.column_types[*col1].nullable + { is_not_nulls.push(MirScalarExpr::Column(*col1).call_is_null().not()) } } @@ -425,7 +426,7 @@ fn list_replacements_join( // Each unique key could be a semijoin candidate. // We want to check that the join equivalences exactly match the key, // and then transcribe the corresponding columns in the other input. - if distinct_on_keys_of(&inputs[1], &rtl) { + if distinct_on_keys_of(&inputs[1].typ(), &rtl) { let columns = ltr .iter() .map(|(k0, k1)| (*k0, *k0, *k1)) @@ -455,7 +456,7 @@ fn list_replacements_join( // Each unique key could be a semijoin candidate. // We want to check that the join equivalences exactly match the key, // and then transcribe the corresponding columns in the other input. - if distinct_on_keys_of(&inputs[0], <r) { + if distinct_on_keys_of(&inputs[0].typ(), <r) { let columns = ltr .iter() .map(|(k0, k1)| (*k1, *k0, *k0)) @@ -487,10 +488,9 @@ fn list_replacements_join( results } -/// True iff some unique key of `input` is contained in the keys of `map`. -fn distinct_on_keys_of(expr: &MirRelationExpr, map: &BTreeMap) -> bool { - expr.typ() - .keys +/// True iff some unique key of `typ` is contained in the keys of `map`. +fn distinct_on_keys_of(typ: &RelationType, map: &BTreeMap) -> bool { + typ.keys .iter() .any(|key| key.iter().all(|k| map.contains_key(k))) }