Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have Equivalences::minimize take Option<Slice> #30635

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/transform/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ mod explain {
if config.equivalences {
for (expr, equivs) in std::iter::zip(
subtree_refs.iter(),
derived.results::<Equivalences>().unwrap().into_iter(),
derived.results::<Equivalences>().into_iter(),
) {
let analyses = annotations.entry(expr).or_default();
analyses.equivalences = Some(match equivs.as_ref() {
Expand Down
24 changes: 13 additions & 11 deletions src/transform/src/analysis/equivalences.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,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 @@ -239,7 +239,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 @@ -268,8 +268,10 @@ impl Analysis for Equivalences {
MirRelationExpr::ArrangeBy { .. } => results.get(index - 1).unwrap().clone(),
};

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

Expand Down Expand Up @@ -455,15 +457,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 @@ -504,7 +506,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 @@ -645,7 +647,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 @@ -798,7 +800,7 @@ impl EquivalenceClasses {
classes,
remap: Default::default(),
};
equivalences.minimize(&None);
equivalences.minimize(None);
equivalences
}

Expand All @@ -810,7 +812,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 @@ -894,7 +896,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
Loading