Skip to content

Commit 3b3fdea

Browse files
ggevayggevay
committed
optimizer: add enable_less_reduce_in_eqprop feature flag
1 parent e9e4f10 commit 3b3fdea

File tree

5 files changed

+60
-13
lines changed

5 files changed

+60
-13
lines changed

‎src/repr/src/optimize.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ optimizer_feature_flags!({
124124
enable_projection_pushdown_after_relation_cse: bool,
125125
// See the feature flag of the same name.
126126
enable_let_prefix_extraction: bool,
127+
// See the feature flag of the same name.
128+
enable_less_reduce_in_eqprop: bool,
127129
});
128130

129131
/// A trait used to implement layered config construction.

‎src/sql/src/plan/statement/ddl.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4654,7 +4654,9 @@ pub fn unplan_create_cluster(
46544654
enable_join_prioritize_arranged,
46554655
enable_projection_pushdown_after_relation_cse,
46564656
enable_let_prefix_extraction: _,
4657+
enable_less_reduce_in_eqprop: _,
46574658
} = optimizer_feature_overrides;
4659+
// The ones from above that don't occur below are not wired up to cluster features.
46584660
let features_extracted = ClusterFeatureExtracted {
46594661
// Seen is ignored when unplanning.
46604662
seen: Default::default(),

‎src/sql/src/plan/statement/dml.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ impl TryFrom<ExplainPlanOptionExtracted> for ExplainConfig {
435435
enable_projection_pushdown_after_relation_cse: v
436436
.enable_projection_pushdown_after_relation_cse,
437437
enable_let_prefix_extraction: Default::default(),
438+
enable_less_reduce_in_eqprop: Default::default(),
438439
},
439440
})
440441
}

‎src/sql/src/session/vars/definitions.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,6 +2209,12 @@ feature_flags!(
22092209
default: true,
22102210
enable_for_item_parsing: false,
22112211
},
2212+
{
2213+
name: enable_less_reduce_in_eqprop,
2214+
desc: "Run MSE::reduce in EquivalencePropagation only if reduce_expr changed something.",
2215+
default: true,
2216+
enable_for_item_parsing: false,
2217+
},
22122218
);
22132219

22142220
impl From<&super::SystemVars> for OptimizerFeatures {
@@ -2228,6 +2234,7 @@ impl From<&super::SystemVars> for OptimizerFeatures {
22282234
enable_projection_pushdown_after_relation_cse: vars
22292235
.enable_projection_pushdown_after_relation_cse(),
22302236
enable_let_prefix_extraction: vars.enable_let_prefix_extraction(),
2237+
enable_less_reduce_in_eqprop: vars.enable_less_reduce_in_eqprop(),
22312238
}
22322239
}
22332240
}

‎src/transform/src/equivalence_propagation.rs‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ impl crate::Transform for EquivalencePropagation {
7272
derived,
7373
EquivalenceClasses::default(),
7474
&mut get_equivalences,
75+
ctx,
7576
);
7677

7778
// Trace the plan as the result of `equivalence_propagation` before potentially applying
@@ -116,6 +117,7 @@ impl EquivalencePropagation {
116117
derived: DerivedView,
117118
mut outer_equivalences: EquivalenceClasses,
118119
get_equivalences: &mut BTreeMap<Id, EquivalenceClasses>,
120+
ctx: &mut TransformCtx,
119121
) {
120122
// TODO: The top-down traversal can be coded as a worklist, with arguments tupled and enqueued.
121123
// This has the potential to do a lot more cloning (of `outer_equivalences`), and some care is needed
@@ -194,22 +196,40 @@ impl EquivalencePropagation {
194196
let body = children_rev.next().unwrap();
195197
let value = children_rev.next().unwrap();
196198

197-
self.apply(body.0, body.1, outer_equivalences.clone(), get_equivalences);
199+
self.apply(
200+
body.0,
201+
body.1,
202+
outer_equivalences.clone(),
203+
get_equivalences,
204+
ctx,
205+
);
198206

199207
// We expect to find `id` in `get_equivalences`, as otherwise the binding is
200208
// not referenced and can be removed.
201209
if let Some(equivalences) = get_equivalences.get(&Id::Local(id)) {
202-
self.apply(value.0, value.1, equivalences.clone(), get_equivalences);
210+
self.apply(
211+
value.0,
212+
value.1,
213+
equivalences.clone(),
214+
get_equivalences,
215+
ctx,
216+
);
203217
}
204218
}
205219
MirRelationExpr::LetRec { .. } => {
206220
let mut child_iter = expr.children_mut().rev().zip(derived.children_rev());
207221
// Continue in `body` with the outer equivalences.
208222
let (body, view) = child_iter.next().unwrap();
209-
self.apply(body, view, outer_equivalences, get_equivalences);
223+
self.apply(body, view, outer_equivalences, get_equivalences, ctx);
210224
// Continue recursively, but without the outer equivalences supplied to `body`.
211225
for (child, view) in child_iter {
212-
self.apply(child, view, EquivalenceClasses::default(), get_equivalences);
226+
self.apply(
227+
child,
228+
view,
229+
EquivalenceClasses::default(),
230+
get_equivalences,
231+
ctx,
232+
);
213233
}
214234
}
215235
MirRelationExpr::Project { input, outputs } => {
@@ -220,6 +240,7 @@ impl EquivalencePropagation {
220240
derived.last_child(),
221241
outer_equivalences,
222242
get_equivalences,
243+
ctx,
223244
);
224245
}
225246
MirRelationExpr::Map { input, scalars } => {
@@ -242,7 +263,7 @@ impl EquivalencePropagation {
242263
for (index, expr) in scalars.iter_mut().enumerate() {
243264
let reducer = input_equivalences.reducer();
244265
let changed = reducer.reduce_expr(expr);
245-
if changed {
266+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
246267
expr.reduce(&input_types[..(input_arity + index)]);
247268
}
248269
// Introduce the fact relating the mapped expression and corresponding column.
@@ -263,6 +284,7 @@ impl EquivalencePropagation {
263284
derived.last_child(),
264285
outer_equivalences,
265286
get_equivalences,
287+
ctx,
266288
);
267289
}
268290
}
@@ -281,7 +303,7 @@ impl EquivalencePropagation {
281303
let reducer = input_equivalences.reducer();
282304
for expr in exprs.iter_mut() {
283305
let changed = reducer.reduce_expr(expr);
284-
if changed {
306+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
285307
expr.reduce(input_types.as_ref().unwrap());
286308
}
287309
}
@@ -295,6 +317,7 @@ impl EquivalencePropagation {
295317
derived.last_child(),
296318
outer_equivalences,
297319
get_equivalences,
320+
ctx,
298321
);
299322
}
300323
}
@@ -314,7 +337,7 @@ impl EquivalencePropagation {
314337
let reducer = input_equivalences.reducer();
315338
for expr in predicates.iter_mut() {
316339
let changed = reducer.reduce_expr(expr);
317-
if changed {
340+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
318341
expr.reduce(input_types.as_ref().unwrap());
319342
}
320343
}
@@ -331,6 +354,7 @@ impl EquivalencePropagation {
331354
derived.last_child(),
332355
outer_equivalences,
333356
get_equivalences,
357+
ctx,
334358
);
335359
}
336360
}
@@ -411,7 +435,7 @@ impl EquivalencePropagation {
411435
let old = expr.clone();
412436
let changed = reducer.reduce_expr(expr);
413437
let acceptable_sub = literal_domination(&old, expr);
414-
if changed {
438+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
415439
expr.reduce(input_types.as_ref().unwrap());
416440
}
417441
if !acceptable_sub && !literal_domination(&old, expr) {
@@ -448,7 +472,7 @@ impl EquivalencePropagation {
448472
}
449473
}
450474
push_equivalences.project(columns..(columns + child_arity));
451-
self.apply(expr, child, push_equivalences, get_equivalences);
475+
self.apply(expr, child, push_equivalences, get_equivalences, ctx);
452476

453477
columns += child_arity;
454478
}
@@ -482,7 +506,7 @@ impl EquivalencePropagation {
482506
let old_key = key.clone();
483507
let changed = reducer.reduce_expr(key);
484508
let acceptable_sub = literal_domination(&old_key, key);
485-
if changed {
509+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
486510
key.reduce(input_type.as_ref().unwrap());
487511
}
488512
if !acceptable_sub && !literal_domination(&old_key, key) {
@@ -491,7 +515,7 @@ impl EquivalencePropagation {
491515
}
492516
for aggr in aggregates.iter_mut() {
493517
let changed = reducer.reduce_expr(&mut aggr.expr);
494-
if changed {
518+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
495519
aggr.expr.reduce(input_type.as_ref().unwrap());
496520
}
497521
// A count expression over a non-null expression can discard the expression.
@@ -529,6 +553,7 @@ impl EquivalencePropagation {
529553
derived.last_child(),
530554
outer_equivalences,
531555
get_equivalences,
556+
ctx,
532557
);
533558
}
534559
MirRelationExpr::TopK {
@@ -554,7 +579,7 @@ impl EquivalencePropagation {
554579
let old_expr = expr.clone();
555580
let changed = reducer.reduce_expr(expr);
556581
let acceptable_sub = literal_domination(&old_expr, expr);
557-
if changed {
582+
if changed || !ctx.features.enable_less_reduce_in_eqprop {
558583
expr.reduce(input_types.as_ref().unwrap());
559584
}
560585
if !acceptable_sub && !literal_domination(&old_expr, expr) {
@@ -571,6 +596,7 @@ impl EquivalencePropagation {
571596
derived.last_child(),
572597
outer_equivalences,
573598
get_equivalences,
599+
ctx,
574600
);
575601
}
576602
MirRelationExpr::Negate { input } => {
@@ -579,6 +605,7 @@ impl EquivalencePropagation {
579605
derived.last_child(),
580606
outer_equivalences,
581607
get_equivalences,
608+
ctx,
582609
);
583610
}
584611
MirRelationExpr::Threshold { input } => {
@@ -587,11 +614,18 @@ impl EquivalencePropagation {
587614
derived.last_child(),
588615
outer_equivalences,
589616
get_equivalences,
617+
ctx,
590618
);
591619
}
592620
MirRelationExpr::Union { .. } => {
593621
for (child, derived) in expr.children_mut().rev().zip(derived.children_rev()) {
594-
self.apply(child, derived, outer_equivalences.clone(), get_equivalences);
622+
self.apply(
623+
child,
624+
derived,
625+
outer_equivalences.clone(),
626+
get_equivalences,
627+
ctx,
628+
);
595629
}
596630
}
597631
MirRelationExpr::ArrangeBy { input, .. } => {
@@ -601,6 +635,7 @@ impl EquivalencePropagation {
601635
derived.last_child(),
602636
outer_equivalences,
603637
get_equivalences,
638+
ctx,
604639
);
605640
}
606641
}

0 commit comments

Comments
 (0)