Skip to content

Commit

Permalink
Have Equivalences::minimize take Option<Slice>
Browse files Browse the repository at this point in the history
  • Loading branch information
frankmcsherry committed Nov 26, 2024
1 parent 1bd4533 commit 9dacfe4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 16 deletions.
24 changes: 13 additions & 11 deletions src/transform/src/analysis/equivalences.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl Analysis for Equivalences {

// Having added classes to `equivalences`, we should minimize the classes to fold the
// information in before applying the `project`, to set it up for success.
equivalences.minimize(&None);
equivalences.minimize(None);

// Grab a copy of the equivalences with key columns added to use in aggregate reasoning.
let extended = equivalences.clone();
Expand All @@ -237,7 +237,7 @@ impl Analysis for Equivalences {
MirScalarExpr::column(input_arity + group_key.len()),
aggregate.expr.clone(),
]);
temp_equivs.minimize(&None);
temp_equivs.minimize(None);
temp_equivs.project(input_arity..(input_arity + group_key.len() + 1));
let columns = (0..group_key.len())
.chain(std::iter::once(group_key.len() + index))
Expand Down Expand Up @@ -266,8 +266,10 @@ impl Analysis for Equivalences {
MirRelationExpr::ArrangeBy { .. } => results.get(index - 1).unwrap().clone(),
};

let expr_type = depends.results::<RelationType>().unwrap()[index].clone();
equivalences.as_mut().map(|e| e.minimize(&expr_type));
let expr_type = depends.results::<RelationType>().unwrap()[index].as_ref();
equivalences
.as_mut()
.map(|e| e.minimize(expr_type.map(|x| &x[..])));
equivalences
}

Expand Down Expand Up @@ -442,15 +444,15 @@ impl EquivalenceClasses {
/// Update `self` to maintain the same equivalences which potentially reducing along `Ord::le`.
///
/// Informally this means simplifying constraints, removing redundant constraints, and unifying equivalence classes.
pub fn minimize(&mut self, columns: &Option<Vec<ColumnType>>) {
pub fn minimize(&mut self, columns: Option<&[ColumnType]>) {
// Repeatedly, we reduce each of the classes themselves, then unify the classes.
// This should strictly reduce complexity, and reach a fixed point.
// Ideally it is *confluent*, arriving at the same fixed point no matter the order of operations.

// We should not rely on nullability information present in `column_types`. (Doing this
// every time just before calling `reduce` was found to be a bottleneck during incident-217,
// so now we do this nullability tweaking only once here.)
let mut columns = columns.clone();
let mut columns = columns.map(|x| x.to_vec());
let mut nonnull = Vec::new();
if let Some(columns) = columns.as_mut() {
for (index, col) in columns.iter_mut().enumerate() {
Expand Down Expand Up @@ -491,7 +493,7 @@ impl EquivalenceClasses {
// An expression can be simplified, a duplication found, or two classes unified.
let mut stable = false;
while !stable {
stable = !self.minimize_once(&columns);
stable = !self.minimize_once(columns.as_ref().map(|x| &x[..]));
}

// Termination detection.
Expand Down Expand Up @@ -632,7 +634,7 @@ impl EquivalenceClasses {
/// 1. Performs per-expression reduction, including the class structure to replace subexpressions.
/// 2. Applies idiom detection to e.g. unpack expressions equivalence to literal true or false.
/// 3. Restores the equivalence class invariants.
fn minimize_once(&mut self, columns: &Option<Vec<ColumnType>>) -> bool {
fn minimize_once(&mut self, columns: Option<&[ColumnType]>) -> bool {
// 1. Reduce each expression
//
// This reduction first looks for subexpression substitutions that can be performed,
Expand Down Expand Up @@ -785,7 +787,7 @@ impl EquivalenceClasses {
classes,
remap: Default::default(),
};
equivalences.minimize(&None);
equivalences.minimize(None);
equivalences
}

Expand All @@ -797,7 +799,7 @@ impl EquivalenceClasses {
}
}
self.remap.clear();
self.minimize(&None);
self.minimize(None);
}

/// Subject the constraints to the column projection, reworking and removing equivalences.
Expand Down Expand Up @@ -881,7 +883,7 @@ impl EquivalenceClasses {
]);
}
self.remap.clear();
self.minimize(&None);
self.minimize(None);
}

/// True if any equivalence class contains two distinct non-error literals.
Expand Down
10 changes: 5 additions & 5 deletions src/transform/src/equivalence_propagation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl EquivalencePropagation {
}
}

outer_equivalences.minimize(expr_type);
outer_equivalences.minimize(expr_type.as_ref().map(|x| &x[..]));
if outer_equivalences.unsatisfiable() {
expr.take_safely();
return;
Expand Down Expand Up @@ -239,7 +239,7 @@ impl EquivalencePropagation {
expr.clone(),
MirScalarExpr::column(input_arity + index),
]);
input_equivalences.minimize(&Some(input_types.clone()));
input_equivalences.minimize(Some(input_types));
}
let input_arity = *derived
.last_child()
Expand Down Expand Up @@ -309,7 +309,7 @@ impl EquivalencePropagation {
mz_repr::ScalarType::Bool,
));
outer_equivalences.classes.push(class);
outer_equivalences.minimize(input_types);
outer_equivalences.minimize(input_types.as_ref().map(|x| &x[..]));
self.apply(
input,
derived.last_child(),
Expand Down Expand Up @@ -368,7 +368,7 @@ impl EquivalencePropagation {
if let Some(mut equivalences) = equivalences {
let permutation = (columns..(columns + child_arity)).collect::<Vec<_>>();
equivalences.permute(&permutation);
equivalences.minimize(&input_types);
equivalences.minimize(input_types.as_ref().map(|x| &x[..]));
input_equivalences.push(equivalences);
}
columns += child_arity;
Expand Down Expand Up @@ -409,7 +409,7 @@ impl EquivalencePropagation {
col.nullable = true;
}
}
join_equivalences.minimize(&input_types);
join_equivalences.minimize(input_types.as_ref().map(|x| &x[..]));

// Revisit each child, determining the information to present to it, and recurring.
let mut columns = 0;
Expand Down

0 comments on commit 9dacfe4

Please sign in to comment.