From a54d5ec7b1498269662230f7dffc3c053d407507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Taylor?= Date: Mon, 26 Aug 2024 21:52:11 +0200 Subject: [PATCH] Performance improvements - Planner (#16631) Signed-off-by: Andres Taylor --- .../planbuilder/operators/aggregator.go | 3 -- go/vt/vtgate/planbuilder/operators/delete.go | 35 ++++++++----------- .../planbuilder/operators/dml_with_input.go | 3 -- .../planbuilder/operators/fk_cascade.go | 3 -- .../vtgate/planbuilder/operators/fk_verify.go | 3 -- .../vtgate/planbuilder/operators/operator.go | 3 +- .../vtgate/planbuilder/operators/rewriters.go | 11 +++--- go/vt/vtgate/planbuilder/operators/route.go | 18 ++++++---- go/vt/vtgate/planbuilder/operators/update.go | 8 ++--- go/vt/vtgate/semantics/dependencies.go | 29 +++++++++++---- go/vt/vtgate/semantics/real_table.go | 25 +++++++++---- 11 files changed, 77 insertions(+), 64 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/aggregator.go b/go/vt/vtgate/planbuilder/operators/aggregator.go index cfeaf1c8cdc..63c21ba2bce 100644 --- a/go/vt/vtgate/planbuilder/operators/aggregator.go +++ b/go/vt/vtgate/planbuilder/operators/aggregator.go @@ -80,9 +80,6 @@ func (a *Aggregator) Inputs() []Operator { } func (a *Aggregator) SetInputs(operators []Operator) { - if len(operators) != 1 { - panic(fmt.Sprintf("unexpected number of operators as input in aggregator: %d", len(operators))) - } a.Source = operators[0] } diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index 5bbf5218bd7..ef8403e5603 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -45,9 +45,6 @@ func (d *Delete) Inputs() []Operator { } func (d *Delete) SetInputs(inputs []Operator) { - if len(inputs) != 1 { - panic(vterrors.VT13001("unexpected number of inputs for Delete operator")) - } d.Source = inputs[0] } @@ -260,16 +257,16 @@ func createDeleteOperator(ctx *plancontext.PlanningContext, del *sqlparser.Delet Ignore: del.Ignore, Target: targetTbl, OwnedVindexQuery: ovq, - Source: op, }, } if del.Limit != nil { - addOrdering(ctx, del.OrderBy, delOp) delOp.Source = &Limit{ - Source: delOp.Source, + Source: addOrdering(ctx, op, del.OrderBy), AST: del.Limit, } + } else { + delOp.Source = op } return sqc.getRootOperator(delOp, nil), vTbl @@ -302,26 +299,24 @@ func makeColName(col sqlparser.IdentifierCI, table TargetTable, isMultiTbl bool) return sqlparser.NewColName(col.String()) } -func addOrdering(ctx *plancontext.PlanningContext, orderBy sqlparser.OrderBy, op Operator) { +func addOrdering(ctx *plancontext.PlanningContext, op Operator, orderBy sqlparser.OrderBy) Operator { es := &expressionSet{} - ordering := &Ordering{} - ordering.SetInputs(op.Inputs()) - for _, order := range orderBy { - if sqlparser.IsNull(order.Expr) { - // ORDER BY null can safely be ignored - continue - } - if !es.add(ctx, order.Expr) { + var order []OrderBy + for _, ord := range orderBy { + if sqlparser.IsNull(ord.Expr) || !es.add(ctx, ord.Expr) { + // ORDER BY null, or expression repeated can safely be ignored continue } - ordering.Order = append(ordering.Order, OrderBy{ - Inner: sqlparser.Clone(order), - SimplifiedExpr: order.Expr, + + order = append(order, OrderBy{ + Inner: sqlparser.Clone(ord), + SimplifiedExpr: ord.Expr, }) } - if len(ordering.Order) > 0 { - op.SetInputs([]Operator{ordering}) + if len(order) == 0 { + return op } + return &Ordering{Source: op, Order: order} } func updateQueryGraphWithSource(ctx *plancontext.PlanningContext, input Operator, tblID semantics.TableSet, vTbl *vindexes.Table) *vindexes.Table { diff --git a/go/vt/vtgate/planbuilder/operators/dml_with_input.go b/go/vt/vtgate/planbuilder/operators/dml_with_input.go index 3843e2f3fa8..720056f1964 100644 --- a/go/vt/vtgate/planbuilder/operators/dml_with_input.go +++ b/go/vt/vtgate/planbuilder/operators/dml_with_input.go @@ -50,9 +50,6 @@ func (d *DMLWithInput) Inputs() []Operator { } func (d *DMLWithInput) SetInputs(inputs []Operator) { - if len(inputs) < 2 { - panic("unexpected number of inputs for DMLWithInput operator") - } d.Source = inputs[0] d.DML = inputs[1:] } diff --git a/go/vt/vtgate/planbuilder/operators/fk_cascade.go b/go/vt/vtgate/planbuilder/operators/fk_cascade.go index f24b59ca5ab..0aff5b3bea2 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_cascade.go +++ b/go/vt/vtgate/planbuilder/operators/fk_cascade.go @@ -61,9 +61,6 @@ func (fkc *FkCascade) Inputs() []Operator { // SetInputs implements the Operator interface func (fkc *FkCascade) SetInputs(operators []Operator) { - if len(operators) < 2 { - panic("incorrect count of inputs for FkCascade") - } fkc.Parent = operators[0] fkc.Selection = operators[1] for idx, operator := range operators { diff --git a/go/vt/vtgate/planbuilder/operators/fk_verify.go b/go/vt/vtgate/planbuilder/operators/fk_verify.go index 8275a8d462f..a27f88f3335 100644 --- a/go/vt/vtgate/planbuilder/operators/fk_verify.go +++ b/go/vt/vtgate/planbuilder/operators/fk_verify.go @@ -52,9 +52,6 @@ func (fkv *FkVerify) Inputs() []Operator { // SetInputs implements the Operator interface func (fkv *FkVerify) SetInputs(operators []Operator) { fkv.Input = operators[0] - if len(fkv.Verify) != len(operators)-1 { - panic("mismatched number of verify inputs") - } for i := 1; i < len(operators); i++ { fkv.Verify[i-1].Op = operators[i] } diff --git a/go/vt/vtgate/planbuilder/operators/operator.go b/go/vt/vtgate/planbuilder/operators/operator.go index f1a38974c93..76797aee906 100644 --- a/go/vt/vtgate/planbuilder/operators/operator.go +++ b/go/vt/vtgate/planbuilder/operators/operator.go @@ -54,7 +54,8 @@ type ( // Inputs returns the inputs for this operator Inputs() []Operator - // SetInputs changes the inputs for this op + // SetInputs changes the inputs for this op. + // We don't need to check the size of the inputs, as the planner will ensure that the inputs are correct SetInputs([]Operator) // AddPredicate is used to push predicates. It pushed it as far down as is possible in the tree. diff --git a/go/vt/vtgate/planbuilder/operators/rewriters.go b/go/vt/vtgate/planbuilder/operators/rewriters.go index ab9fe66e368..a5743e001e1 100644 --- a/go/vt/vtgate/planbuilder/operators/rewriters.go +++ b/go/vt/vtgate/planbuilder/operators/rewriters.go @@ -227,7 +227,7 @@ func bottomUp( } if anythingChanged.Changed() { - root = root.Clone(newInputs) + root.SetInputs(newInputs) } newOp, treeIdentity := rewriter(root, rootID, isRoot) @@ -257,11 +257,7 @@ func breakableTopDown( } } - if anythingChanged.Changed() { - return newOp, NoRewrite, nil - } - - return newOp.Clone(newInputs), anythingChanged, nil + return newOp, anythingChanged, nil } // topDown is a helper function that recursively traverses the operator tree from the @@ -301,7 +297,8 @@ func topDown( } if anythingChanged != NoRewrite { - return root.Clone(newInputs), anythingChanged + root.SetInputs(newInputs) + return root, anythingChanged } return root, NoRewrite diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index 1c91077c2e4..25e3f610a04 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -19,12 +19,10 @@ package operators import ( "fmt" - "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/slice" "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vtenv" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/evalengine" @@ -121,7 +119,7 @@ func UpdateRoutingLogic(ctx *plancontext.PlanningContext, expr sqlparser.Expr, r } nr := &NoneRouting{keyspace: ks} - if isConstantFalse(ctx.VSchema.Environment(), expr, ctx.VSchema.ConnCollation()) { + if isConstantFalse(ctx, expr) { return nr } @@ -165,11 +163,19 @@ func UpdateRoutingLogic(ctx *plancontext.PlanningContext, expr sqlparser.Expr, r // isConstantFalse checks whether this predicate can be evaluated at plan-time. If it returns `false` or `null`, // we know that the query will not return anything, and this can be used to produce better plans -func isConstantFalse(env *vtenv.Environment, expr sqlparser.Expr, collation collations.ID) bool { +func isConstantFalse(ctx *plancontext.PlanningContext, expr sqlparser.Expr) bool { + if !ctx.SemTable.RecursiveDeps(expr).IsEmpty() { + // we have column dependencies, so we can be pretty sure + // we won't be able to use the evalengine to check if this is constant false + return false + } + env := ctx.VSchema.Environment() + collation := ctx.VSchema.ConnCollation() eenv := evalengine.EmptyExpressionEnv(env) eexpr, err := evalengine.Translate(expr, &evalengine.Config{ - Collation: collation, - Environment: env, + Collation: collation, + Environment: env, + NoCompilation: true, }) if err != nil { return false diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index e843155246c..b4f0a37914e 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -66,9 +66,6 @@ func (u *Update) Inputs() []Operator { } func (u *Update) SetInputs(inputs []Operator) { - if len(inputs) != 1 { - panic(vterrors.VT13001("unexpected number of inputs for Update operator")) - } u.Source = inputs[0] } @@ -388,7 +385,6 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U Ignore: updStmt.Ignore, Target: targetTbl, OwnedVindexQuery: ovq, - Source: op, }, Assignments: assignments, ChangedVindexValues: cvv, @@ -397,7 +393,9 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U } if len(updStmt.OrderBy) > 0 { - addOrdering(ctx, updStmt.OrderBy, updOp) + updOp.Source = addOrdering(ctx, op, updStmt.OrderBy) + } else { + updOp.Source = op } if updStmt.Limit != nil { diff --git a/go/vt/vtgate/semantics/dependencies.go b/go/vt/vtgate/semantics/dependencies.go index 70167ff02fc..42b7a918384 100644 --- a/go/vt/vtgate/semantics/dependencies.go +++ b/go/vt/vtgate/semantics/dependencies.go @@ -17,6 +17,8 @@ limitations under the License. package semantics import ( + "fmt" + querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/evalengine" @@ -29,6 +31,7 @@ type ( empty() bool get(col *sqlparser.ColName) (dependency, error) merge(other dependencies, allowMulti bool) dependencies + debugString() string } dependency struct { certain bool @@ -100,6 +103,10 @@ func (u *uncertain) merge(d dependencies, _ bool) dependencies { } } +func (u *uncertain) debugString() string { + return fmt.Sprintf("uncertain: %v %v %s", u.direct, u.recursive, u.typ.Type().String()) +} + func (c *certain) empty() bool { return false } @@ -117,26 +124,34 @@ func (c *certain) merge(d dependencies, allowMulti bool) dependencies { if d.recursive == c.recursive { return c } - c.direct = c.direct.Merge(d.direct) - c.recursive = c.recursive.Merge(d.recursive) + + res := createCertain(c.direct.Merge(d.direct), c.recursive.Merge(d.recursive), c.typ) if !allowMulti { - c.err = true + res.err = true } - return c + return res } return c } -func (n *nothing) empty() bool { +func (c *certain) debugString() string { + return fmt.Sprintf("certain: %v %v %s", c.direct, c.recursive, c.typ.Type().String()) +} + +func (*nothing) empty() bool { return true } -func (n *nothing) get(*sqlparser.ColName) (dependency, error) { +func (*nothing) get(*sqlparser.ColName) (dependency, error) { return dependency{certain: true}, nil } -func (n *nothing) merge(d dependencies, _ bool) dependencies { +func (*nothing) merge(d dependencies, _ bool) dependencies { return d } + +func (*nothing) debugString() string { + return "nothing" +} diff --git a/go/vt/vtgate/semantics/real_table.go b/go/vt/vtgate/semantics/real_table.go index 399395a9edf..33c15b749f9 100644 --- a/go/vt/vtgate/semantics/real_table.go +++ b/go/vt/vtgate/semantics/real_table.go @@ -37,23 +37,36 @@ type RealTable struct { VindexHint *sqlparser.IndexHint isInfSchema bool collationEnv *collations.Environment + cache map[string]dependencies } var _ TableInfo = (*RealTable)(nil) // dependencies implements the TableInfo interface -func (r *RealTable) dependencies(colName string, org originable) (dependencies, error) { - ts := org.tableSetFor(r.ASTNode) - for _, info := range r.getColumns(false /* ignoreInvisbleCol */) { - if strings.EqualFold(info.Name, colName) { - return createCertain(ts, ts, info.Type), nil +func (r *RealTable) dependencies(colName string, org originable) (deps dependencies, err error) { + var myID *TableSet + if r.cache == nil { + r.cache = make(map[string]dependencies) + ts := org.tableSetFor(r.ASTNode) + myID = &ts + for _, info := range r.getColumns(false /* ignoreInvisbleCol */) { + r.cache[strings.ToLower(info.Name)] = createCertain(ts, ts, info.Type) } } + if deps, ok := r.cache[strings.ToLower(colName)]; ok { + return deps, nil + } + if r.authoritative() { return ¬hing{}, nil } - return createUncertain(ts, ts), nil + + if myID == nil { + ts := org.tableSetFor(r.ASTNode) + myID = &ts + } + return createUncertain(*myID, *myID), nil } // GetTables implements the TableInfo interface