refactor: remove rewrite methods from the logicalPlan interface

Signed-off-by: Andres Taylor <[email protected]>
vitessio · Oct 17, 2023 · 23d7e94 · 23d7e94
1 parent 00a7ac9
commit 23d7e94
Show file tree

Hide file tree

Showing 17 changed files with 21 additions and 204 deletions.
diff --git a/go/vt/vtgate/planbuilder/concatenate.go b/go/vt/vtgate/planbuilder/concatenate.go
@@ -17,7 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -54,15 +53,6 @@ func (c *concatenate) Primitive() engine.Primitive {
 	return engine.NewConcatenate(sources, c.noNeedToTypeCheck)
 }
 
-// Rewrite implements the logicalPlan interface
-func (c *concatenate) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != len(c.sources) {
-		return vterrors.VT13001("concatenate: wrong number of inputs")
-	}
-	c.sources = inputs
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (c *concatenate) ContainsTables() semantics.TableSet {
 	var tableSet semantics.TableSet

diff --git a/go/vt/vtgate/planbuilder/distinct.go b/go/vt/vtgate/planbuilder/distinct.go
@@ -17,7 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 )
 
@@ -62,15 +61,6 @@ func (d *distinct) Primitive() engine.Primitive {
 	}
 }
 
-// Rewrite implements the logicalPlan interface
-func (d *distinct) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 1 {
-		return vterrors.VT13001("distinct: wrong number of inputs")
-	}
-	d.input = inputs[0]
-	return nil
-}
-
 // Inputs implements the logicalPlan interface
 func (d *distinct) Inputs() []logicalPlan {
 	return []logicalPlan{d.input}

diff --git a/go/vt/vtgate/planbuilder/fk_cascade.go b/go/vt/vtgate/planbuilder/fk_cascade.go
@@ -17,7 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -58,16 +57,6 @@ func (fkc *fkCascade) Wireup(ctx *plancontext.PlanningContext) error {
 	return fkc.selection.Wireup(ctx)
 }
 
-// Rewrite implements the logicalPlan interface
-func (fkc *fkCascade) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 2 {
-		return vterrors.VT13001("fkCascade: wrong number of inputs")
-	}
-	fkc.parent = inputs[0]
-	fkc.selection = inputs[1]
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (fkc *fkCascade) ContainsTables() semantics.TableSet {
 	return fkc.parent.ContainsTables()

diff --git a/go/vt/vtgate/planbuilder/fk_verify.go b/go/vt/vtgate/planbuilder/fk_verify.go
@@ -17,7 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -70,18 +69,6 @@ func (fkc *fkVerify) Wireup(ctx *plancontext.PlanningContext) error {
 	return fkc.input.Wireup(ctx)
 }
 
-// Rewrite implements the logicalPlan interface
-func (fkc *fkVerify) Rewrite(inputs ...logicalPlan) error {
-	if len(fkc.verify) != len(inputs)-1 {
-		return vterrors.VT13001("fkVerify: wrong number of inputs")
-	}
-	fkc.input = inputs[0]
-	for i := 1; i < len(inputs); i++ {
-		fkc.verify[i-1].verify = inputs[i]
-	}
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (fkc *fkVerify) ContainsTables() semantics.TableSet {
 	return fkc.input.ContainsTables()

diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go
@@ -52,7 +52,7 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm
 	if ks != nil {
 		if tables[0].AutoIncrement == nil && !ctx.SemTable.ForeignKeysPresent() {
 			plan := insertUnshardedShortcut(insStmt, ks, tables)
-			plan = pushCommentDirectivesOnPlan(plan, insStmt)
+			setCommentDirectivesOnPlan(plan, insStmt)
 			return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil
 		}
 	}
@@ -138,10 +138,6 @@ func (i *insert) Inputs() []logicalPlan {
 	return []logicalPlan{i.source}
 }
 
-func (i *insert) Rewrite(inputs ...logicalPlan) error {
-	panic("does not expect insert to get rewrite call")
-}
-
 func (i *insert) ContainsTables() semantics.TableSet {
 	panic("does not expect insert to get contains tables call")
 }
diff --git a/go/vt/vtgate/planbuilder/join.go b/go/vt/vtgate/planbuilder/join.go
@@ -17,10 +17,7 @@ limitations under the License.
 package planbuilder
 
 import (
-	"fmt"
-
 	"vitess.io/vitess/go/vt/sqlparser"
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -75,16 +72,6 @@ func (j *join) Inputs() []logicalPlan {
 	return []logicalPlan{j.Left, j.Right}
 }
 
-// Rewrite implements the logicalPlan interface
-func (j *join) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 2 {
-		return vterrors.VT13001(fmt.Sprintf("wrong number of children in join rewrite, got: %d, expect: 2", len(inputs)))
-	}
-	j.Left = inputs[0]
-	j.Right = inputs[1]
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (j *join) ContainsTables() semantics.TableSet {
 	return j.Left.ContainsTables().Merge(j.Right.ContainsTables())

diff --git a/go/vt/vtgate/planbuilder/logical_plan.go b/go/vt/vtgate/planbuilder/logical_plan.go
@@ -17,9 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"fmt"
-
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -39,49 +36,11 @@ type logicalPlan interface {
 	// Inputs are the children of this plan
 	Inputs() []logicalPlan
 
-	// Rewrite replaces the inputs of this plan with the ones provided
-	Rewrite(inputs ...logicalPlan) error
-
 	// ContainsTables keeps track which query tables are being solved by this logical plan
 	// This is only applicable for plans that have been built with the Gen4 planner
 	ContainsTables() semantics.TableSet
 }
 
-type planVisitor func(logicalPlan) (bool, logicalPlan, error)
-
-func visit(node logicalPlan, visitor planVisitor) (logicalPlan, error) {
-	if visitor != nil {
-		kontinue, newNode, err := visitor(node)
-		if err != nil {
-			return nil, err
-		}
-		if !kontinue {
-			return newNode, nil
-		}
-		node = newNode
-	}
-	inputs := node.Inputs()
-	rewrite := false
-	for i, input := range inputs {
-		newInput, err := visit(input, visitor)
-		if err != nil {
-			return nil, err
-		}
-		if newInput != input {
-			rewrite = true
-		}
-		inputs[i] = newInput
-	}
-	if rewrite {
-		err := node.Rewrite(inputs...)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return node, nil
-}
-
 // -------------------------------------------------------------------------
 
 // logicalPlanCommon implements some common functionality of builders.
@@ -103,15 +62,6 @@ func (bc *logicalPlanCommon) Wireup(ctx *plancontext.PlanningContext) error {
 	return bc.input.Wireup(ctx)
 }
 
-// Rewrite implements the logicalPlan interface
-func (bc *logicalPlanCommon) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 1 {
-		return vterrors.VT13001(fmt.Sprintf("builderCommon: wrong number of inputs, got: %d, expect: 1", len(inputs)))
-	}
-	bc.input = inputs[0]
-	return nil
-}
-
 // Inputs implements the logicalPlan interface
 func (bc *logicalPlanCommon) Inputs() []logicalPlan {
 	return []logicalPlan{bc.input}

diff --git a/go/vt/vtgate/planbuilder/planner.go b/go/vt/vtgate/planbuilder/planner.go
@@ -44,34 +44,29 @@ func gen4Planner(query string, plannerVersion querypb.ExecuteOptions_PlannerVers
 	}
 }
 
-// pushCommentDirectivesOnPlan adds comments to queries
+// setCommentDirectivesOnPlan adds comments to queries
 // TODO: this should move to the operator side of planning
-func pushCommentDirectivesOnPlan(plan logicalPlan, stmt sqlparser.Statement) logicalPlan {
+func setCommentDirectivesOnPlan(plan logicalPlan, stmt sqlparser.Statement) {
 	var directives *sqlparser.CommentDirectives
 	cmt, ok := stmt.(sqlparser.Commented)
-	if ok {
-		directives = cmt.GetParsedComments().Directives()
-		scatterAsWarns := directives.IsSet(sqlparser.DirectiveScatterErrorsAsWarnings)
-		timeout := queryTimeout(directives)
-		multiShardAutoCommit := directives.IsSet(sqlparser.DirectiveMultiShardAutocommit)
-
-		if scatterAsWarns || timeout > 0 || multiShardAutoCommit {
-			_, _ = visit(plan, func(logicalPlan logicalPlan) (bool, logicalPlan, error) {
-				switch plan := logicalPlan.(type) {
-				case *route:
-					plan.eroute.ScatterErrorsAsWarnings = scatterAsWarns
-					plan.eroute.QueryTimeout = timeout
-				case *primitiveWrapper:
-					setDirective(plan.prim, multiShardAutoCommit, timeout)
-				case *insert:
-					setDirective(plan.eInsert, multiShardAutoCommit, timeout)
-				}
-				return true, logicalPlan, nil
-			})
-		}
+	if !ok {
+		return
 	}
 
-	return plan
+	directives = cmt.GetParsedComments().Directives()
+	scatterAsWarns := directives.IsSet(sqlparser.DirectiveScatterErrorsAsWarnings)
+	timeout := queryTimeout(directives)
+	multiShardAutoCommit := directives.IsSet(sqlparser.DirectiveMultiShardAutocommit)
+
+	switch plan := plan.(type) {
+	case *route:
+		plan.eroute.ScatterErrorsAsWarnings = scatterAsWarns
+		plan.eroute.QueryTimeout = timeout
+	case *primitiveWrapper:
+		setDirective(plan.prim, multiShardAutoCommit, timeout)
+	case *insert:
+		setDirective(plan.eInsert, multiShardAutoCommit, timeout)
+	}
 }
 
 func setDirective(prim engine.Primitive, msac bool, timeout int) {

diff --git a/go/vt/vtgate/planbuilder/primitive_wrapper.go b/go/vt/vtgate/planbuilder/primitive_wrapper.go
@@ -17,7 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -40,10 +39,6 @@ func (p *primitiveWrapper) Inputs() []logicalPlan {
 	return nil
 }
 
-func (p *primitiveWrapper) Rewrite(...logicalPlan) error {
-	return vterrors.VT13001("cannot rewrite")
-}
-
 func (p *primitiveWrapper) ContainsTables() semantics.TableSet {
 	return semantics.EmptyTableSet()
 }

diff --git a/go/vt/vtgate/planbuilder/projection.go b/go/vt/vtgate/planbuilder/projection.go
@@ -17,8 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"fmt"
-
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
@@ -52,15 +50,6 @@ func (p *projection) Inputs() []logicalPlan {
 	return []logicalPlan{p.source}
 }
 
-// Rewrite implements the logicalPlan interface
-func (p *projection) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 1 {
-		return vterrors.VT13001(fmt.Sprintf("wrong number of inputs, got: %d; expected: %d", len(inputs), 1))
-	}
-	p.source = inputs[0]
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (p *projection) ContainsTables() semantics.TableSet {
 	return p.source.ContainsTables()

diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go
@@ -142,14 +142,6 @@ func (rb *route) prepareTheAST() {
 	}, rb.Select)
 }
 
-// Rewrite implements the logicalPlan interface
-func (rb *route) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 0 {
-		return vterrors.VT13001("route: wrong number of inputs")
-	}
-	return nil
-}
-
 // Inputs implements the logicalPlan interface
 func (rb *route) Inputs() []logicalPlan {
 	return []logicalPlan{}

diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go
@@ -214,7 +214,7 @@ func newBuildSelectPlan(
 		if err != nil {
 			return nil, nil, err
 		}
-		plan = pushCommentDirectivesOnPlan(plan, selStmt)
+		setCommentDirectivesOnPlan(plan, selStmt)
 		return plan, tablesUsed, err
 	}
 

diff --git a/go/vt/vtgate/planbuilder/semi_join.go b/go/vt/vtgate/planbuilder/semi_join.go
@@ -18,7 +18,6 @@ package planbuilder
 
 import (
 	"vitess.io/vitess/go/vt/sqlparser"
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -69,16 +68,6 @@ func (ps *semiJoin) Wireup(ctx *plancontext.PlanningContext) error {
 	return ps.rhs.Wireup(ctx)
 }
 
-// Rewrite implements the logicalPlan interface
-func (ps *semiJoin) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 2 {
-		return vterrors.VT13001("semiJoin: wrong number of inputs")
-	}
-	ps.lhs = inputs[0]
-	ps.rhs = inputs[1]
-	return nil
-}
-
 // ContainsTables implements the logicalPlan interface
 func (ps *semiJoin) ContainsTables() semantics.TableSet {
 	return ps.lhs.ContainsTables().Merge(ps.rhs.ContainsTables())

diff --git a/go/vt/vtgate/planbuilder/sql_calc_found_rows.go b/go/vt/vtgate/planbuilder/sql_calc_found_rows.go
@@ -17,9 +17,6 @@ limitations under the License.
 package planbuilder
 
 import (
-	"fmt"
-
-	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/semantics"
@@ -59,16 +56,6 @@ func (s *sqlCalcFoundRows) Primitive() engine.Primitive {
 	}
 }
 
-// Rewrite implements the logicalPlan interface
-func (s *sqlCalcFoundRows) Rewrite(inputs ...logicalPlan) error {
-	if len(inputs) != 2 {
-		return vterrors.VT13001(fmt.Sprintf("wrong number of inputs for SQL_CALC_FOUND_ROWS: %d", len(inputs)))
-	}
-	s.LimitQuery = inputs[0]
-	s.CountQuery = inputs[1]
-	return nil
-}
-
 // Inputs implements the logicalPlan interface
 func (s *sqlCalcFoundRows) Inputs() []logicalPlan {
 	return []logicalPlan{s.LimitQuery, s.CountQuery}