Skip to content

Commit 8499840

Browse files
authored
chore(engine): Eliminate no-op Merge/SortMerge nodes during plan optimization (#19238)
Merge and SortMerge nodes with 1 or less children are effectively no-op nodes and can safely be removed from the plan during the optimization phase. Signed-off-by: Christian Haudum <[email protected]>
1 parent 35816aa commit 8499840

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

pkg/engine/planner/physical/optimizer.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,26 @@ func (r *removeNoopFilter) apply(node Node) bool {
3535

3636
var _ rule = (*removeNoopFilter)(nil)
3737

38+
// removeNoopMerge is a rule that removes merge/sortmerge nodes with only a single input
39+
type removeNoopMerge struct {
40+
plan *Plan
41+
}
42+
43+
// apply implements rule.
44+
func (r *removeNoopMerge) apply(node Node) bool {
45+
changed := false
46+
switch node := node.(type) {
47+
case *Merge, *SortMerge:
48+
if len(r.plan.Children(node)) <= 1 {
49+
r.plan.eliminateNode(node)
50+
changed = true
51+
}
52+
}
53+
return changed
54+
}
55+
56+
var _ rule = (*removeNoopMerge)(nil)
57+
3858
// predicatePushdown is a rule that moves down filter predicates to the scan nodes.
3959
type predicatePushdown struct {
4060
plan *Plan

pkg/engine/planner/physical/optimizer_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package physical
22

33
import (
4+
"fmt"
45
"sort"
56
"testing"
67
"time"
@@ -104,6 +105,7 @@ func dummyPlan() *Plan {
104105
}
105106

106107
func TestOptimizer(t *testing.T) {
108+
107109
t.Run("noop", func(t *testing.T) {
108110
plan := dummyPlan()
109111
optimizations := []*optimization{
@@ -561,6 +563,43 @@ func TestOptimizer(t *testing.T) {
561563
expected := PrintAsTree(expectedPlan)
562564
require.Equal(t, expected, actual)
563565
})
566+
567+
t.Run("cleanup no-op merge nodes", func(t *testing.T) {
568+
plan := func() *Plan {
569+
plan := &Plan{}
570+
limit := plan.addNode(&Limit{id: "limit"})
571+
merge := plan.addNode(&Merge{id: "merge"})
572+
sortmerge := plan.addNode(&Merge{id: "sortmerge"})
573+
scan := plan.addNode(&DataObjScan{id: "scan"})
574+
575+
_ = plan.addEdge(Edge{Parent: limit, Child: merge})
576+
_ = plan.addEdge(Edge{Parent: merge, Child: sortmerge})
577+
_ = plan.addEdge(Edge{Parent: sortmerge, Child: scan})
578+
return plan
579+
}()
580+
581+
optimizations := []*optimization{
582+
newOptimization("cleanup", plan).withRules(
583+
&removeNoopMerge{plan},
584+
),
585+
}
586+
587+
o := newOptimizer(plan, optimizations)
588+
o.optimize(plan.Roots()[0])
589+
actual := PrintAsTree(plan)
590+
591+
optimized := func() *Plan {
592+
plan := &Plan{}
593+
limit := plan.addNode(&Limit{id: "limit"})
594+
scan := plan.addNode(&DataObjScan{id: "scan"})
595+
596+
_ = plan.addEdge(Edge{Parent: limit, Child: scan})
597+
return plan
598+
}()
599+
600+
expected := PrintAsTree(optimized)
601+
require.Equal(t, expected, actual, fmt.Sprintf("Expected:\n%s\nActual:\n%s\n", expected, actual))
602+
})
564603
}
565604

566605
func TestProjectionPushdown_PushesRequestedKeysToParseNodes(t *testing.T) {

pkg/engine/planner/physical/planner.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,14 +381,17 @@ func (p *Planner) Optimize(plan *Plan) (*Plan, error) {
381381
optimizations := []*optimization{
382382
newOptimization("PredicatePushdown", plan).withRules(
383383
&predicatePushdown{plan: plan},
384-
&removeNoopFilter{plan: plan},
385384
),
386385
newOptimization("LimitPushdown", plan).withRules(
387386
&limitPushdown{plan: plan},
388387
),
389388
newOptimization("ProjectionPushdown", plan).withRules(
390389
&projectionPushdown{plan: plan},
391390
),
391+
newOptimization("Cleanup", plan).withRules(
392+
&removeNoopFilter{plan: plan},
393+
&removeNoopMerge{plan: plan},
394+
),
392395
}
393396
optimizer := newOptimizer(plan, optimizations)
394397
optimizer.optimize(root)

0 commit comments

Comments
 (0)