From f2d07c6c17cd9175153b45ae56c5525cd3315de1 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Sat, 25 Mar 2023 12:07:02 +0100 Subject: [PATCH 01/10] planner refactor: push columns by AliasedExpr instead of Expr Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/operators/apply_join.go | 16 ++++++++-------- go/vt/vtgate/planbuilder/operators/derived.go | 6 +++--- go/vt/vtgate/planbuilder/operators/filter.go | 2 +- .../planbuilder/operators/horizon_planning.go | 4 ++++ go/vt/vtgate/planbuilder/operators/operator.go | 2 +- go/vt/vtgate/planbuilder/operators/ops/op.go | 2 +- go/vt/vtgate/planbuilder/operators/route.go | 4 ++-- .../planbuilder/operators/subquery_planning.go | 4 ++-- go/vt/vtgate/planbuilder/operators/table.go | 4 ++-- go/vt/vtgate/planbuilder/operators/vindex.go | 5 ++--- 10 files changed, 26 insertions(+), 23 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 2968d463b1c..85280a763ee 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -122,7 +122,7 @@ func (a *ApplyJoin) AddJoinPredicate(ctx *plancontext.PlanningContext, expr sqlp return err } for i, col := range cols { - offset, err := a.LHS.AddColumn(ctx, col) + offset, err := a.LHS.AddColumn(ctx, aeWrap(col)) if err != nil { return err } @@ -140,10 +140,10 @@ func (a *ApplyJoin) AddJoinPredicate(ctx *plancontext.PlanningContext, expr sqlp return nil } -func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (int, error) { +func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { // first check if we already are passing through this expression for i, existing := range a.ColumnsAST { - if ctx.SemTable.EqualsExpr(existing, expr) { + if ctx.SemTable.EqualsExpr(existing, expr.Expr) { return i, nil } } @@ -151,7 +151,7 @@ func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.E lhs := TableID(a.LHS) rhs := TableID(a.RHS) both := lhs.Merge(rhs) - deps := ctx.SemTable.RecursiveDeps(expr) + deps := ctx.SemTable.RecursiveDeps(expr.Expr) // if we get here, it's a new expression we are dealing with. // We need to decide if we can push it all on either side, @@ -164,18 +164,18 @@ func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.E } a.Columns = append(a.Columns, -offset-1) case deps.IsSolvedBy(both): - bvNames, lhsExprs, rhsExpr, err := BreakExpressionInLHSandRHS(ctx, expr, lhs) + bvNames, lhsExprs, rhsExpr, err := BreakExpressionInLHSandRHS(ctx, expr.Expr, lhs) if err != nil { return 0, err } for i, lhsExpr := range lhsExprs { - offset, err := a.LHS.AddColumn(ctx, lhsExpr) + offset, err := a.LHS.AddColumn(ctx, aeWrap(lhsExpr)) if err != nil { return 0, err } a.Vars[bvNames[i]] = offset } - expr = rhsExpr + expr = aeWrap(rhsExpr) fallthrough // now we just pass the rest to the RHS of the join case deps.IsSolvedBy(rhs): offset, err := a.RHS.AddColumn(ctx, expr) @@ -188,6 +188,6 @@ func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.E } // the expression wasn't already there - let's add it - a.ColumnsAST = append(a.ColumnsAST, expr) + a.ColumnsAST = append(a.ColumnsAST, expr.Expr) return len(a.Columns) - 1, nil } diff --git a/go/vt/vtgate/planbuilder/operators/derived.go b/go/vt/vtgate/planbuilder/operators/derived.go index 06fa8a3f7af..7238545f1f4 100644 --- a/go/vt/vtgate/planbuilder/operators/derived.go +++ b/go/vt/vtgate/planbuilder/operators/derived.go @@ -132,8 +132,8 @@ func (d *Derived) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser. return d, nil } -func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (int, error) { - col, ok := expr.(*sqlparser.ColName) +func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { + col, ok := expr.Expr.(*sqlparser.ColName) if !ok { return 0, vterrors.VT13001("cannot push non-colname expression to a derived table") } @@ -148,7 +148,7 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.Exp d.Columns = append(d.Columns, col) // add it to the source if we were not already passing it through if i <= -1 { - _, err := d.Source.AddColumn(ctx, sqlparser.NewColName(col.Name.String())) + _, err := d.Source.AddColumn(ctx, aeWrap(sqlparser.NewColName(col.Name.String()))) if err != nil { return 0, err } diff --git a/go/vt/vtgate/planbuilder/operators/filter.go b/go/vt/vtgate/planbuilder/operators/filter.go index d28511dbe86..3d36dfa5379 100644 --- a/go/vt/vtgate/planbuilder/operators/filter.go +++ b/go/vt/vtgate/planbuilder/operators/filter.go @@ -77,7 +77,7 @@ func (f *Filter) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.E return f, nil } -func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (int, error) { +func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { return f.Source.AddColumn(ctx, expr) } diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index 61d6b56f4f7..1400cfaeade 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -76,3 +76,7 @@ func planSingleRoute(rb *Route, horizon *Horizon) (ops.Operator, rewrite.VisitRu rb.Source, horizon.Source = horizon, rb.Source return rb, rewrite.SkipChildren, nil } + +func aeWrap(e sqlparser.Expr) *sqlparser.AliasedExpr { + return &sqlparser.AliasedExpr{Expr: e} +} diff --git a/go/vt/vtgate/planbuilder/operators/operator.go b/go/vt/vtgate/planbuilder/operators/operator.go index 35a3d2af91d..cd811d52421 100644 --- a/go/vt/vtgate/planbuilder/operators/operator.go +++ b/go/vt/vtgate/planbuilder/operators/operator.go @@ -88,7 +88,7 @@ func (noInputs) Inputs() []ops.Operator { } // AddColumn implements the Operator interface -func (noColumns) AddColumn(*plancontext.PlanningContext, sqlparser.Expr) (int, error) { +func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr) (int, error) { return 0, vterrors.VT13001("the noColumns operator cannot accept columns") } diff --git a/go/vt/vtgate/planbuilder/operators/ops/op.go b/go/vt/vtgate/planbuilder/operators/ops/op.go index 4deeb5cee1e..574837edcef 100644 --- a/go/vt/vtgate/planbuilder/operators/ops/op.go +++ b/go/vt/vtgate/planbuilder/operators/ops/op.go @@ -39,7 +39,7 @@ type ( // AddColumn tells an operator to also output an additional column specified. // The offset to the column is returned. - AddColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (int, error) + AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) } // PhysicalOperator means that this operator is ready to be turned into a logical plan diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index 276de2a23c1..f8d8748a055 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -517,8 +517,8 @@ func (r *Route) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Ex return r, err } -func (r *Route) AddColumn(ctx *plancontext.PlanningContext, e sqlparser.Expr) (int, error) { - return r.Source.AddColumn(ctx, e) +func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { + return r.Source.AddColumn(ctx, expr) } // TablesUsed returns tables used by MergedWith routes, which are not included diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index 890dae5005f..605234f4b28 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -361,7 +361,7 @@ func rewriteColumnsInSubqueryOpForJoin( return true } // if it does not exist, then push this as an output column there and add it to the joinVars - offset, err := resultInnerOp.AddColumn(ctx, node) + offset, err := resultInnerOp.AddColumn(ctx, aeWrap(node)) if err != nil { rewriteError = err return false @@ -426,7 +426,7 @@ func createCorrelatedSubqueryOp( bindVars[node] = bindVar // if it does not exist, then push this as an output column in the outerOp and add it to the joinVars - offset, err := resultOuterOp.AddColumn(ctx, node) + offset, err := resultOuterOp.AddColumn(ctx, aeWrap(node)) if err != nil { rewriteError = err return true diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index 593dfe3ec7a..53bb9b42d8b 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -67,8 +67,8 @@ func (to *Table) AddPredicate(_ *plancontext.PlanningContext, expr sqlparser.Exp return newFilter(to, expr), nil } -func (to *Table) AddColumn(_ *plancontext.PlanningContext, e sqlparser.Expr) (int, error) { - return addColumn(to, e) +func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { + return addColumn(to, expr.Expr) } func (to *Table) GetColumns() []*sqlparser.ColName { diff --git a/go/vt/vtgate/planbuilder/operators/vindex.go b/go/vt/vtgate/planbuilder/operators/vindex.go index 0c0d6976fb5..c62081044f3 100644 --- a/go/vt/vtgate/planbuilder/operators/vindex.go +++ b/go/vt/vtgate/planbuilder/operators/vindex.go @@ -66,8 +66,8 @@ func (v *Vindex) Clone([]ops.Operator) ops.Operator { var _ ops.PhysicalOperator = (*Vindex)(nil) -func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr sqlparser.Expr) (int, error) { - return addColumn(v, expr) +func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { + return addColumn(v, expr.Expr) } func (v *Vindex) GetColumns() []*sqlparser.ColName { @@ -77,7 +77,6 @@ func (v *Vindex) AddCol(col *sqlparser.ColName) { v.Columns = append(v.Columns, col) } -// checkValid implements the Operator interface func (v *Vindex) CheckValid() error { if len(v.Table.Predicates) == 0 { return vterrors.VT12001(VindexUnsupported + " (where clause missing)") From 459735e3a1539e6e056689f10aaea079fd35e272 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Sat, 25 Mar 2023 14:21:28 +0100 Subject: [PATCH 02/10] planner refactor: allow AddColumnd to return new operator Signed-off-by: Andres Taylor --- .../planbuilder/operators/apply_join.go | 50 +++++++++++++------ go/vt/vtgate/planbuilder/operators/derived.go | 13 ++--- go/vt/vtgate/planbuilder/operators/filter.go | 9 +++- .../vtgate/planbuilder/operators/operator.go | 4 +- go/vt/vtgate/planbuilder/operators/ops/op.go | 2 +- go/vt/vtgate/planbuilder/operators/route.go | 9 +++- .../operators/subquery_planning.go | 6 ++- go/vt/vtgate/planbuilder/operators/table.go | 9 +++- go/vt/vtgate/planbuilder/operators/vindex.go | 10 +++- 9 files changed, 78 insertions(+), 34 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 85280a763ee..7f849a3131e 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -122,7 +122,7 @@ func (a *ApplyJoin) AddJoinPredicate(ctx *plancontext.PlanningContext, expr sqlp return err } for i, col := range cols { - offset, err := a.LHS.AddColumn(ctx, aeWrap(col)) + offset, err := a.pushColLeft(ctx, aeWrap(col)) if err != nil { return err } @@ -140,11 +140,28 @@ func (a *ApplyJoin) AddJoinPredicate(ctx *plancontext.PlanningContext, expr sqlp return nil } -func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { +func (a *ApplyJoin) pushColLeft(ctx *plancontext.PlanningContext, e *sqlparser.AliasedExpr) (int, error) { + newLHS, offset, err := a.LHS.AddColumn(ctx, e) + if err != nil { + return 0, err + } + a.LHS = newLHS + return offset, nil +} +func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser.AliasedExpr) (int, error) { + newRHS, offset, err := a.RHS.AddColumn(ctx, e) + if err != nil { + return 0, err + } + a.RHS = newRHS + return offset, nil +} + +func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { // first check if we already are passing through this expression for i, existing := range a.ColumnsAST { if ctx.SemTable.EqualsExpr(existing, expr.Expr) { - return i, nil + return a, i, nil } } @@ -158,36 +175,39 @@ func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser. // or if we have to break the expression into left and right parts switch { case deps.IsSolvedBy(lhs): - offset, err := a.LHS.AddColumn(ctx, expr) + offset, err := a.pushColLeft(ctx, expr) if err != nil { - return 0, err + return nil, 0, err } a.Columns = append(a.Columns, -offset-1) + case deps.IsSolvedBy(rhs): + offset, err := a.pushColRight(ctx, expr) + if err != nil { + return nil, 0, err + } + a.Columns = append(a.Columns, offset+1) case deps.IsSolvedBy(both): bvNames, lhsExprs, rhsExpr, err := BreakExpressionInLHSandRHS(ctx, expr.Expr, lhs) if err != nil { - return 0, err + return nil, 0, err } for i, lhsExpr := range lhsExprs { - offset, err := a.LHS.AddColumn(ctx, aeWrap(lhsExpr)) + offset, err := a.pushColLeft(ctx, aeWrap(lhsExpr)) if err != nil { - return 0, err + return nil, 0, err } a.Vars[bvNames[i]] = offset } - expr = aeWrap(rhsExpr) - fallthrough // now we just pass the rest to the RHS of the join - case deps.IsSolvedBy(rhs): - offset, err := a.RHS.AddColumn(ctx, expr) + offset, err := a.pushColRight(ctx, aeWrap(rhsExpr)) if err != nil { - return 0, err + return nil, 0, err } a.Columns = append(a.Columns, offset+1) default: - return 0, vterrors.VT13002(sqlparser.String(expr)) + return nil, 0, vterrors.VT13002(sqlparser.String(expr)) } // the expression wasn't already there - let's add it a.ColumnsAST = append(a.ColumnsAST, expr.Expr) - return len(a.Columns) - 1, nil + return a, len(a.Columns) - 1, nil } diff --git a/go/vt/vtgate/planbuilder/operators/derived.go b/go/vt/vtgate/planbuilder/operators/derived.go index 7238545f1f4..d967ac58e5e 100644 --- a/go/vt/vtgate/planbuilder/operators/derived.go +++ b/go/vt/vtgate/planbuilder/operators/derived.go @@ -132,15 +132,15 @@ func (d *Derived) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser. return d, nil } -func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { +func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { col, ok := expr.Expr.(*sqlparser.ColName) if !ok { - return 0, vterrors.VT13001("cannot push non-colname expression to a derived table") + return nil, 0, vterrors.VT13001("cannot push non-colname expression to a derived table") } i, err := d.findOutputColumn(col) if err != nil { - return 0, err + return nil, 0, err } var pos int d.ColumnsOffset, pos = addToIntSlice(d.ColumnsOffset, i) @@ -148,12 +148,13 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Al d.Columns = append(d.Columns, col) // add it to the source if we were not already passing it through if i <= -1 { - _, err := d.Source.AddColumn(ctx, aeWrap(sqlparser.NewColName(col.Name.String()))) + newSrc, _, err := d.Source.AddColumn(ctx, aeWrap(sqlparser.NewColName(col.Name.String()))) if err != nil { - return 0, err + return nil, 0, err } + d.Source = newSrc } - return pos, nil + return d, pos, nil } func addToIntSlice(columnOffset []int, valToAdd int) ([]int, int) { diff --git a/go/vt/vtgate/planbuilder/operators/filter.go b/go/vt/vtgate/planbuilder/operators/filter.go index 3d36dfa5379..9ac1277947b 100644 --- a/go/vt/vtgate/planbuilder/operators/filter.go +++ b/go/vt/vtgate/planbuilder/operators/filter.go @@ -77,8 +77,13 @@ func (f *Filter) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.E return f, nil } -func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { - return f.Source.AddColumn(ctx, expr) +func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { + newSrc, offset, err := f.Source.AddColumn(ctx, expr) + if err != nil { + return nil, 0, err + } + f.Source = newSrc + return f, offset, nil } func (f *Filter) Compact(*plancontext.PlanningContext) (ops.Operator, rewrite.TreeIdentity, error) { diff --git a/go/vt/vtgate/planbuilder/operators/operator.go b/go/vt/vtgate/planbuilder/operators/operator.go index cd811d52421..9e0e892f2af 100644 --- a/go/vt/vtgate/planbuilder/operators/operator.go +++ b/go/vt/vtgate/planbuilder/operators/operator.go @@ -88,8 +88,8 @@ func (noInputs) Inputs() []ops.Operator { } // AddColumn implements the Operator interface -func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr) (int, error) { - return 0, vterrors.VT13001("the noColumns operator cannot accept columns") +func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr) (ops.Operator, int, error) { + return nil, 0, vterrors.VT13001("the noColumns operator cannot accept columns") } // AddPredicate implements the Operator interface diff --git a/go/vt/vtgate/planbuilder/operators/ops/op.go b/go/vt/vtgate/planbuilder/operators/ops/op.go index 574837edcef..34d9ba369a9 100644 --- a/go/vt/vtgate/planbuilder/operators/ops/op.go +++ b/go/vt/vtgate/planbuilder/operators/ops/op.go @@ -39,7 +39,7 @@ type ( // AddColumn tells an operator to also output an additional column specified. // The offset to the column is returned. - AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) + AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (Operator, int, error) } // PhysicalOperator means that this operator is ready to be turned into a logical plan diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index f8d8748a055..6881683a87c 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -517,8 +517,13 @@ func (r *Route) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Ex return r, err } -func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { - return r.Source.AddColumn(ctx, expr) +func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { + newSrc, offset, err := r.Source.AddColumn(ctx, expr) + if err != nil { + return nil, 0, err + } + r.Source = newSrc + return r, offset, nil } // TablesUsed returns tables used by MergedWith routes, which are not included diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index 605234f4b28..f436778c38e 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -361,11 +361,12 @@ func rewriteColumnsInSubqueryOpForJoin( return true } // if it does not exist, then push this as an output column there and add it to the joinVars - offset, err := resultInnerOp.AddColumn(ctx, aeWrap(node)) + newInnerOp, offset, err := resultInnerOp.AddColumn(ctx, aeWrap(node)) if err != nil { rewriteError = err return false } + resultInnerOp = newInnerOp outerTree.Vars[bindVar] = offset return true }) @@ -426,11 +427,12 @@ func createCorrelatedSubqueryOp( bindVars[node] = bindVar // if it does not exist, then push this as an output column in the outerOp and add it to the joinVars - offset, err := resultOuterOp.AddColumn(ctx, aeWrap(node)) + newOuterOp, offset, err := resultOuterOp.AddColumn(ctx, aeWrap(node)) if err != nil { rewriteError = err return true } + resultOuterOp = newOuterOp lhsCols = append(lhsCols, node) vars[bindVar] = offset return true diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index 53bb9b42d8b..d928ab19e5f 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -67,8 +67,13 @@ func (to *Table) AddPredicate(_ *plancontext.PlanningContext, expr sqlparser.Exp return newFilter(to, expr), nil } -func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { - return addColumn(to, expr.Expr) +func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { + offset, err := addColumn(to, expr.Expr) + if err != nil { + return nil, 0, err + } + + return to, offset, nil } func (to *Table) GetColumns() []*sqlparser.ColName { diff --git a/go/vt/vtgate/planbuilder/operators/vindex.go b/go/vt/vtgate/planbuilder/operators/vindex.go index c62081044f3..e28fe302cc9 100644 --- a/go/vt/vtgate/planbuilder/operators/vindex.go +++ b/go/vt/vtgate/planbuilder/operators/vindex.go @@ -66,8 +66,14 @@ func (v *Vindex) Clone([]ops.Operator) ops.Operator { var _ ops.PhysicalOperator = (*Vindex)(nil) -func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (int, error) { - return addColumn(v, expr.Expr) +func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { + offset, err := addColumn(v, expr.Expr) + if err != nil { + return nil, 0, err + } + + return v, offset, nil + } func (v *Vindex) GetColumns() []*sqlparser.ColName { From 205091be63c0983111986c47a4a59b5f4f74a4a7 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Sat, 25 Mar 2023 14:46:53 +0100 Subject: [PATCH 03/10] planner comment: added comment Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/operators/horizon.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/horizon.go b/go/vt/vtgate/planbuilder/operators/horizon.go index 7bbe3eb9e98..74c34480caf 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon.go +++ b/go/vt/vtgate/planbuilder/operators/horizon.go @@ -22,8 +22,12 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" ) -// Horizon is an operator we use until we decide how to handle the source to the horizon. -// It contains information about the planning we have to do after deciding how we will send the query to the tablets. +// Horizon is an operator that allows us to postpone planning things like SELECT/GROUP BY/ORDER BY/LIMIT until later. +// It contains information about the planning we have to do after deciding how we will send the query to the tablets. // If we are able to push down the Horizon under a route, we don't have to plan these things separately and can +// just copy over the AST constructs to the query being sent to a tablet. +// If we are not able to push it down, this operator needs to be split up into smaller +// Project/Aggregate/Sort/Limit operations, some which can be pushed down, +// and some that have to be evaluated at the vtgate level. type Horizon struct { Source ops.Operator Select sqlparser.SelectStatement From 82e17459e29e143edc087c3e5cf611b183811f42 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 09:17:45 +0200 Subject: [PATCH 04/10] planbuilder: handle simpler cases of horizons on the operators Signed-off-by: Andres Taylor --- .../planbuilder/operators/apply_join.go | 9 ++- go/vt/vtgate/planbuilder/operators/derived.go | 12 ++- go/vt/vtgate/planbuilder/operators/filter.go | 4 +- go/vt/vtgate/planbuilder/operators/helpers.go | 2 +- .../planbuilder/operators/horizon_planning.go | 76 ++++++++++++++----- .../vtgate/planbuilder/operators/operator.go | 4 +- go/vt/vtgate/planbuilder/operators/ops/op.go | 2 +- .../operators/rewrite/rewriters.go | 42 ++++++++-- go/vt/vtgate/planbuilder/operators/route.go | 4 +- .../planbuilder/operators/route_planning.go | 2 +- .../operators/subquery_planning.go | 4 +- go/vt/vtgate/planbuilder/operators/table.go | 15 ++-- go/vt/vtgate/planbuilder/operators/vindex.go | 4 +- 13 files changed, 133 insertions(+), 47 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 7f849a3131e..5c9287f57a4 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -141,7 +141,7 @@ func (a *ApplyJoin) AddJoinPredicate(ctx *plancontext.PlanningContext, expr sqlp } func (a *ApplyJoin) pushColLeft(ctx *plancontext.PlanningContext, e *sqlparser.AliasedExpr) (int, error) { - newLHS, offset, err := a.LHS.AddColumn(ctx, e) + newLHS, offset, err := a.LHS.AddColumn(ctx, e, true) if err != nil { return 0, err } @@ -149,7 +149,7 @@ func (a *ApplyJoin) pushColLeft(ctx *plancontext.PlanningContext, e *sqlparser.A return offset, nil } func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser.AliasedExpr) (int, error) { - newRHS, offset, err := a.RHS.AddColumn(ctx, e) + newRHS, offset, err := a.RHS.AddColumn(ctx, e, true) if err != nil { return 0, err } @@ -157,9 +157,12 @@ func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser. return offset, nil } -func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { +func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { // first check if we already are passing through this expression for i, existing := range a.ColumnsAST { + if !reuseCol { + break + } if ctx.SemTable.EqualsExpr(existing, expr.Expr) { return a, i, nil } diff --git a/go/vt/vtgate/planbuilder/operators/derived.go b/go/vt/vtgate/planbuilder/operators/derived.go index d967ac58e5e..a5d467be8c2 100644 --- a/go/vt/vtgate/planbuilder/operators/derived.go +++ b/go/vt/vtgate/planbuilder/operators/derived.go @@ -132,12 +132,20 @@ func (d *Derived) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser. return d, nil } -func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { +func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { col, ok := expr.Expr.(*sqlparser.ColName) if !ok { return nil, 0, vterrors.VT13001("cannot push non-colname expression to a derived table") } + if reuseCol { + for offset, column := range d.Columns { + if ctx.SemTable.EqualsExpr(col, column) { + return d, offset, nil + } + } + } + i, err := d.findOutputColumn(col) if err != nil { return nil, 0, err @@ -148,7 +156,7 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Al d.Columns = append(d.Columns, col) // add it to the source if we were not already passing it through if i <= -1 { - newSrc, _, err := d.Source.AddColumn(ctx, aeWrap(sqlparser.NewColName(col.Name.String()))) + newSrc, _, err := d.Source.AddColumn(ctx, aeWrap(sqlparser.NewColName(col.Name.String())), true) if err != nil { return nil, 0, err } diff --git a/go/vt/vtgate/planbuilder/operators/filter.go b/go/vt/vtgate/planbuilder/operators/filter.go index 9ac1277947b..2eee2ac28ea 100644 --- a/go/vt/vtgate/planbuilder/operators/filter.go +++ b/go/vt/vtgate/planbuilder/operators/filter.go @@ -77,8 +77,8 @@ func (f *Filter) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.E return f, nil } -func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { - newSrc, offset, err := f.Source.AddColumn(ctx, expr) +func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + newSrc, offset, err := f.Source.AddColumn(ctx, expr, reuseCol) if err != nil { return nil, 0, err } diff --git a/go/vt/vtgate/planbuilder/operators/helpers.go b/go/vt/vtgate/planbuilder/operators/helpers.go index 3ac028851b4..86fc64d3818 100644 --- a/go/vt/vtgate/planbuilder/operators/helpers.go +++ b/go/vt/vtgate/planbuilder/operators/helpers.go @@ -35,7 +35,7 @@ func Compact(ctx *plancontext.PlanningContext, op ops.Operator) (ops.Operator, e Compact(ctx *plancontext.PlanningContext) (ops.Operator, rewrite.TreeIdentity, error) } - newOp, err := rewrite.BottomUp(op, semantics.EmptyTableSet(), TableID, func(_ semantics.TableSet, op ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { + newOp, err := rewrite.BottomUpAll(op, TableID, func(_ semantics.TableSet, op ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { newOp, ok := op.(compactable) if !ok { return op, rewrite.SameTree, nil diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index 1400cfaeade..e70337f5579 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -21,33 +21,44 @@ import ( "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/rewrite" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" + "vitess.io/vitess/go/vt/vtgate/semantics" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) var errNotHorizonPlanned = vterrors.VT12001("query cannot be fully operator planned") -func planHorizons(ctx *plancontext.PlanningContext, in ops.Operator) (ops.Operator, error) { - return rewrite.TopDown(in, func(in ops.Operator) (ops.Operator, rewrite.TreeIdentity, rewrite.VisitRule, error) { +func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Operator, error) { + stopAtRoute := func(operator ops.Operator) rewrite.VisitRule { + _, isRoute := operator.(*Route) + return rewrite.VisitRule(!isRoute) + } + visitor := func(id semantics.TableSet, in ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { switch in := in.(type) { case *Horizon: - op, visit, err := planHorizon(ctx, in) + op, err := planHorizon(ctx, in, in == root) if err != nil { - return nil, rewrite.SameTree, rewrite.SkipChildren, err + if vterr, ok := err.(*vterrors.VitessError); ok && vterr.ID == "VT13001" { + // we encountered a bug. let's try to back out + return nil, rewrite.SameTree, errNotHorizonPlanned + } + return nil, rewrite.SameTree, err } - return op, rewrite.NewTree, visit, nil - case *Route: - return in, rewrite.SameTree, rewrite.SkipChildren, nil + return op, rewrite.NewTree, nil + case *Derived, *Filter: + return nil, rewrite.SameTree, errNotHorizonPlanned default: - return in, rewrite.SameTree, rewrite.VisitChildren, nil + return in, rewrite.SameTree, nil } - }) + } + return rewrite.BottomUp(root, TableID, visitor, stopAtRoute) } -func planHorizon(ctx *plancontext.PlanningContext, in *Horizon) (ops.Operator, rewrite.VisitRule, error) { +func planHorizon(ctx *plancontext.PlanningContext, in *Horizon, isRoot bool) (ops.Operator, error) { rb, isRoute := in.Source.(*Route) - if !isRoute { - return in, rewrite.VisitChildren, nil + if !isRoute && ctx.SemTable.NotSingleRouteErr != nil { + // If we got here, we don't have a single shard plan + return nil, ctx.SemTable.NotSingleRouteErr } if isRoute && rb.IsSingleShard() && in.Select.GetLimit() == nil { return planSingleRoute(rb, in) @@ -55,26 +66,55 @@ func planHorizon(ctx *plancontext.PlanningContext, in *Horizon) (ops.Operator, r sel, isSel := in.Select.(*sqlparser.Select) if !isSel { - return nil, rewrite.VisitChildren, errNotHorizonPlanned + return nil, errNotHorizonPlanned } qp, err := CreateQPFromSelect(ctx, sel) if err != nil { - return nil, rewrite.VisitChildren, err + return nil, err } needsOrdering := len(qp.OrderExprs) > 0 canShortcut := isRoute && sel.Having == nil && !needsOrdering + _, isDerived := in.Source.(*Derived) + + // if we are at the root, we have to return the columns the user asked for. in all other levels, we reuse as much as possible + canReuseCols := !isRoot - if !qp.NeedsAggregation() && sel.Having == nil && canShortcut && !needsOrdering && !qp.NeedsDistinct() && in.Select.GetLimit() == nil { + switch { + case qp.NeedsAggregation() || sel.Having != nil || sel.Limit != nil || isDerived || needsOrdering || qp.NeedsDistinct(): + return nil, errNotHorizonPlanned + case canShortcut: return planSingleRoute(rb, in) + default: + src := in.Source + for idx, e := range qp.SelectExprs { + expr, err := e.GetAliasedExpr() + if err != nil { + return nil, err + } + if !expr.As.IsEmpty() { + // we are not handling column names correct yet, so let's fail here for now + return nil, errNotHorizonPlanned + } + var offset int + src, offset, err = src.AddColumn(ctx, expr, canReuseCols) + if err != nil { + return nil, err + } + if idx != offset && isRoot { + // if we are returning something different from what the user asked for, + // we need to add an operator on top to clean up the output + return nil, errNotHorizonPlanned + } + } + return src, nil } - return nil, rewrite.VisitChildren, errNotHorizonPlanned } -func planSingleRoute(rb *Route, horizon *Horizon) (ops.Operator, rewrite.VisitRule, error) { +func planSingleRoute(rb *Route, horizon *Horizon) (ops.Operator, error) { rb.Source, horizon.Source = horizon, rb.Source - return rb, rewrite.SkipChildren, nil + return rb, nil } func aeWrap(e sqlparser.Expr) *sqlparser.AliasedExpr { diff --git a/go/vt/vtgate/planbuilder/operators/operator.go b/go/vt/vtgate/planbuilder/operators/operator.go index 9e0e892f2af..d9924779a0d 100644 --- a/go/vt/vtgate/planbuilder/operators/operator.go +++ b/go/vt/vtgate/planbuilder/operators/operator.go @@ -68,7 +68,7 @@ func PlanQuery(ctx *plancontext.PlanningContext, selStmt sqlparser.Statement) (o backup := Clone(op) - op, err = planHorizons(ctx, op) + op, err = planColumns(ctx, op) if err == errNotHorizonPlanned { op = backup } else if err != nil { @@ -88,7 +88,7 @@ func (noInputs) Inputs() []ops.Operator { } // AddColumn implements the Operator interface -func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr) (ops.Operator, int, error) { +func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr, bool) (ops.Operator, int, error) { return nil, 0, vterrors.VT13001("the noColumns operator cannot accept columns") } diff --git a/go/vt/vtgate/planbuilder/operators/ops/op.go b/go/vt/vtgate/planbuilder/operators/ops/op.go index 34d9ba369a9..c3e5d649870 100644 --- a/go/vt/vtgate/planbuilder/operators/ops/op.go +++ b/go/vt/vtgate/planbuilder/operators/ops/op.go @@ -39,7 +39,7 @@ type ( // AddColumn tells an operator to also output an additional column specified. // The offset to the column is returned. - AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (Operator, int, error) + AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (Operator, int, error) } // PhysicalOperator means that this operator is ready to be turned into a logical plan diff --git a/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go b/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go index 30d6891c193..d0c362211a8 100644 --- a/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go +++ b/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go @@ -22,9 +22,13 @@ import ( ) type ( - Func func(semantics.TableSet, ops.Operator) (ops.Operator, TreeIdentity, error) + VisitF func(semantics.TableSet, ops.Operator) (ops.Operator, TreeIdentity, error) + BreakableFunc func(ops.Operator) (ops.Operator, TreeIdentity, VisitRule, error) + // ShouldVisit is used when we want to control which nodes and ancestors to visit and which to skip + ShouldVisit func(ops.Operator) VisitRule + // TreeIdentity tracks modifications to node and expression trees. // Only return SameTree when it is acceptable to return the original // input and discard the returned result as a performance improvement. @@ -57,14 +61,32 @@ func Visit(root ops.Operator, visitor func(ops.Operator) error) error { // BottomUp rewrites an operator tree from the bottom up. BottomUp applies a transformation function to // the given operator tree from the bottom up. Each callback [f] returns a TreeIdentity that is aggregated // into a final output indicating whether the operator tree was changed. -func BottomUp(root ops.Operator, rootID semantics.TableSet, resolveID func(ops.Operator) semantics.TableSet, f Func) (ops.Operator, error) { - op, _, err := bottomUp(root, rootID, resolveID, f) +func BottomUp( + root ops.Operator, + resolveID func(ops.Operator) semantics.TableSet, + visit VisitF, + shouldVisit ShouldVisit, +) (ops.Operator, error) { + op, _, err := bottomUp(root, semantics.EmptyTableSet(), resolveID, visit, shouldVisit) if err != nil { return nil, err } return op, nil } +// BottomUp rewrites an operator tree from the bottom up. BottomUp applies a transformation function to +// the given operator tree from the bottom up. Each callback [f] returns a TreeIdentity that is aggregated +// into a final output indicating whether the operator tree was changed. +func BottomUpAll( + root ops.Operator, + resolveID func(ops.Operator) semantics.TableSet, + visit VisitF, +) (ops.Operator, error) { + return BottomUp(root, resolveID, visit, func(ops.Operator) VisitRule { + return VisitChildren + }) +} + // TopDown applies a transformation function to the given operator tree from the bottom up. = // Each callback [f] returns a TreeIdentity that is aggregated into a final output indicating whether the // operator tree was changed. @@ -74,7 +96,17 @@ func TopDown(in ops.Operator, rewriter BreakableFunc) (ops.Operator, error) { return op, err } -func bottomUp(root ops.Operator, rootID semantics.TableSet, resolveID func(ops.Operator) semantics.TableSet, rewriter Func) (ops.Operator, TreeIdentity, error) { +func bottomUp( + root ops.Operator, + rootID semantics.TableSet, + resolveID func(ops.Operator) semantics.TableSet, + rewriter VisitF, + shouldVisit ShouldVisit, +) (ops.Operator, TreeIdentity, error) { + if !shouldVisit(root) { + return root, SameTree, nil + } + oldInputs := root.Inputs() anythingChanged := false newInputs := make([]ops.Operator, len(oldInputs)) @@ -93,7 +125,7 @@ func bottomUp(root ops.Operator, rootID semantics.TableSet, resolveID func(ops.O if _, isUnion := root.(noLHSTableSet); !isUnion && i > 0 { childID = childID.Merge(resolveID(oldInputs[0])) } - in, changed, err := bottomUp(operator, childID, resolveID, rewriter) + in, changed, err := bottomUp(operator, childID, resolveID, rewriter, shouldVisit) if err != nil { return nil, SameTree, err } diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index 6881683a87c..2abaeea0184 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -517,8 +517,8 @@ func (r *Route) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Ex return r, err } -func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { - newSrc, offset, err := r.Source.AddColumn(ctx, expr) +func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + newSrc, offset, err := r.Source.AddColumn(ctx, expr, reuseCol) if err != nil { return nil, 0, err } diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go index 8386b2e0f5b..1b7bfe5196e 100644 --- a/go/vt/vtgate/planbuilder/operators/route_planning.go +++ b/go/vt/vtgate/planbuilder/operators/route_planning.go @@ -50,7 +50,7 @@ type ( // Here we try to merge query parts into the same route primitives. At the end of this process, // all the operators in the tree are guaranteed to be PhysicalOperators func transformToPhysical(ctx *plancontext.PlanningContext, in ops.Operator) (ops.Operator, error) { - op, err := rewrite.BottomUp(in, semantics.EmptyTableSet(), TableID, func(ts semantics.TableSet, operator ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { + op, err := rewrite.BottomUpAll(in, TableID, func(ts semantics.TableSet, operator ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { switch op := operator.(type) { case *QueryGraph: return optimizeQueryGraph(ctx, op) diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index f436778c38e..1f3cb33bbf9 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -361,7 +361,7 @@ func rewriteColumnsInSubqueryOpForJoin( return true } // if it does not exist, then push this as an output column there and add it to the joinVars - newInnerOp, offset, err := resultInnerOp.AddColumn(ctx, aeWrap(node)) + newInnerOp, offset, err := resultInnerOp.AddColumn(ctx, aeWrap(node), true) if err != nil { rewriteError = err return false @@ -427,7 +427,7 @@ func createCorrelatedSubqueryOp( bindVars[node] = bindVar // if it does not exist, then push this as an output column in the outerOp and add it to the joinVars - newOuterOp, offset, err := resultOuterOp.AddColumn(ctx, aeWrap(node)) + newOuterOp, offset, err := resultOuterOp.AddColumn(ctx, aeWrap(node), true) if err != nil { rewriteError = err return true diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index d928ab19e5f..1a118629dda 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -67,8 +67,8 @@ func (to *Table) AddPredicate(_ *plancontext.PlanningContext, expr sqlparser.Exp return newFilter(to, expr), nil } -func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { - offset, err := addColumn(to, expr.Expr) +func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + offset, err := addColumn(to, expr.Expr, reuseCol) if err != nil { return nil, 0, err } @@ -90,15 +90,18 @@ func (to *Table) TablesUsed() []string { return SingleQualifiedIdentifier(to.VTable.Keyspace, to.VTable.Name) } -func addColumn(op ColNameColumns, e sqlparser.Expr) (int, error) { +func addColumn(op ColNameColumns, e sqlparser.Expr, reuseCol bool) (int, error) { col, ok := e.(*sqlparser.ColName) if !ok { return 0, vterrors.VT13001("cannot push this expression to a table/vindex") } + sqlparser.RemoveKeyspaceFromColName(col) cols := op.GetColumns() - for idx, column := range cols { - if col.Name.Equal(column.Name) { - return idx, nil + if reuseCol { + for idx, column := range cols { + if col.Name.Equal(column.Name) { + return idx, nil + } } } offset := len(cols) diff --git a/go/vt/vtgate/planbuilder/operators/vindex.go b/go/vt/vtgate/planbuilder/operators/vindex.go index e28fe302cc9..14e25857f4a 100644 --- a/go/vt/vtgate/planbuilder/operators/vindex.go +++ b/go/vt/vtgate/planbuilder/operators/vindex.go @@ -66,8 +66,8 @@ func (v *Vindex) Clone([]ops.Operator) ops.Operator { var _ ops.PhysicalOperator = (*Vindex)(nil) -func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr) (ops.Operator, int, error) { - offset, err := addColumn(v, expr.Expr) +func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + offset, err := addColumn(v, expr.Expr, reuseCol) if err != nil { return nil, 0, err } From f018fcbf3397c9b71ffab6ba70c2147b40734fc6 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 09:33:31 +0200 Subject: [PATCH 05/10] planbuilder: cleanups Signed-off-by: Andres Taylor --- .../planbuilder/operators/apply_join.go | 11 ++++---- go/vt/vtgate/planbuilder/operators/helpers.go | 2 +- .../planbuilder/operators/horizon_planning.go | 3 +- .../operators/rewrite/rewriters.go | 28 ++++++++----------- .../planbuilder/operators/route_planning.go | 2 +- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 5c9287f57a4..6f0300c569b 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -159,12 +159,11 @@ func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser. func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { // first check if we already are passing through this expression - for i, existing := range a.ColumnsAST { - if !reuseCol { - break - } - if ctx.SemTable.EqualsExpr(existing, expr.Expr) { - return a, i, nil + if reuseCol { + for i, existing := range a.ColumnsAST { + if ctx.SemTable.EqualsExpr(existing, expr.Expr) { + return a, i, nil + } } } diff --git a/go/vt/vtgate/planbuilder/operators/helpers.go b/go/vt/vtgate/planbuilder/operators/helpers.go index 86fc64d3818..8c43ed1c372 100644 --- a/go/vt/vtgate/planbuilder/operators/helpers.go +++ b/go/vt/vtgate/planbuilder/operators/helpers.go @@ -35,7 +35,7 @@ func Compact(ctx *plancontext.PlanningContext, op ops.Operator) (ops.Operator, e Compact(ctx *plancontext.PlanningContext) (ops.Operator, rewrite.TreeIdentity, error) } - newOp, err := rewrite.BottomUpAll(op, TableID, func(_ semantics.TableSet, op ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { + newOp, err := rewrite.BottomUpAll(op, TableID, func(op ops.Operator, _ semantics.TableSet) (ops.Operator, rewrite.TreeIdentity, error) { newOp, ok := op.(compactable) if !ok { return op, rewrite.SameTree, nil diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index e70337f5579..3057fc0d15f 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -33,7 +33,7 @@ func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Opera _, isRoute := operator.(*Route) return rewrite.VisitRule(!isRoute) } - visitor := func(id semantics.TableSet, in ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { + visitor := func(in ops.Operator, _ semantics.TableSet) (ops.Operator, rewrite.TreeIdentity, error) { switch in := in.(type) { case *Horizon: op, err := planHorizon(ctx, in, in == root) @@ -46,6 +46,7 @@ func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Opera } return op, rewrite.NewTree, nil case *Derived, *Filter: + // TODO we need to do column planning on these return nil, rewrite.SameTree, errNotHorizonPlanned default: return in, rewrite.SameTree, nil diff --git a/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go b/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go index d0c362211a8..1eb3b879bf8 100644 --- a/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go +++ b/go/vt/vtgate/planbuilder/operators/rewrite/rewriters.go @@ -22,9 +22,11 @@ import ( ) type ( - VisitF func(semantics.TableSet, ops.Operator) (ops.Operator, TreeIdentity, error) - - BreakableFunc func(ops.Operator) (ops.Operator, TreeIdentity, VisitRule, error) + // VisitF is the visitor that walks an operator tree + VisitF func( + op ops.Operator, // op is the operator being visited + lhsTables semantics.TableSet, // lhsTables contains the TableSet for all table on the LHS of our parent + ) (ops.Operator, TreeIdentity, error) // ShouldVisit is used when we want to control which nodes and ancestors to visit and which to skip ShouldVisit func(ops.Operator) VisitRule @@ -48,7 +50,7 @@ const ( // Visit allows for the walking of the operator tree. If any error is returned, the walk is aborted func Visit(root ops.Operator, visitor func(ops.Operator) error) error { - _, err := TopDown(root, func(op ops.Operator) (ops.Operator, TreeIdentity, VisitRule, error) { + _, _, err := breakableTopDown(root, func(op ops.Operator) (ops.Operator, TreeIdentity, VisitRule, error) { err := visitor(op) if err != nil { return nil, SameTree, SkipChildren, err @@ -87,15 +89,6 @@ func BottomUpAll( }) } -// TopDown applies a transformation function to the given operator tree from the bottom up. = -// Each callback [f] returns a TreeIdentity that is aggregated into a final output indicating whether the -// operator tree was changed. -// The callback also returns a VisitRule that signals whether the children of this operator should be visited or not -func TopDown(in ops.Operator, rewriter BreakableFunc) (ops.Operator, error) { - op, _, err := breakableTopDown(in, rewriter) - return op, err -} - func bottomUp( root ops.Operator, rootID semantics.TableSet, @@ -120,7 +113,7 @@ func bottomUp( for i, operator := range oldInputs { // We merge the table set of all the LHS above the current root so that we can // send it down to the current RHS. - // We don't want to send the LHS table set to the RHS if the root is an UNION. + // We don't want to send the LHS table set to the RHS if the root is a UNION. // Some operators, like SubQuery, can have multiple child operators on the RHS if _, isUnion := root.(noLHSTableSet); !isUnion && i > 0 { childID = childID.Merge(resolveID(oldInputs[0])) @@ -139,7 +132,7 @@ func bottomUp( root = root.Clone(newInputs) } - newOp, treeIdentity, err := rewriter(rootID, root) + newOp, treeIdentity, err := rewriter(root, rootID) if err != nil { return nil, SameTree, err } @@ -149,7 +142,10 @@ func bottomUp( return newOp, treeIdentity, nil } -func breakableTopDown(in ops.Operator, rewriter BreakableFunc) (ops.Operator, TreeIdentity, error) { +func breakableTopDown( + in ops.Operator, + rewriter func(ops.Operator) (ops.Operator, TreeIdentity, VisitRule, error), +) (ops.Operator, TreeIdentity, error) { newOp, identity, visit, err := rewriter(in) if err != nil || visit == SkipChildren { return newOp, identity, err diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go index 1b7bfe5196e..62b8d7abcf9 100644 --- a/go/vt/vtgate/planbuilder/operators/route_planning.go +++ b/go/vt/vtgate/planbuilder/operators/route_planning.go @@ -50,7 +50,7 @@ type ( // Here we try to merge query parts into the same route primitives. At the end of this process, // all the operators in the tree are guaranteed to be PhysicalOperators func transformToPhysical(ctx *plancontext.PlanningContext, in ops.Operator) (ops.Operator, error) { - op, err := rewrite.BottomUpAll(in, TableID, func(ts semantics.TableSet, operator ops.Operator) (ops.Operator, rewrite.TreeIdentity, error) { + op, err := rewrite.BottomUpAll(in, TableID, func(operator ops.Operator, ts semantics.TableSet) (ops.Operator, rewrite.TreeIdentity, error) { switch op := operator.(type) { case *QueryGraph: return optimizeQueryGraph(ctx, op) From 1215ee7f7cdad296c8219968734067593c196376 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 10:52:19 +0200 Subject: [PATCH 06/10] planbuilder: plan column offsets for Filter on operators Signed-off-by: Andres Taylor --- .../planbuilder/operator_transformers.go | 18 +++++--- go/vt/vtgate/planbuilder/operators/filter.go | 5 +++ .../planbuilder/operators/horizon_planning.go | 44 +++++++++++++++++-- .../planbuilder/operators/route_planning.go | 2 +- .../operators/subquery_planning.go | 6 +-- go/vt/vtgate/semantics/semantic_state.go | 10 +++++ 6 files changed, 71 insertions(+), 14 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index 7ce349b21b5..e3780009c80 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -64,13 +64,19 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i if err != nil { return nil, err } + + predicate := op.FinalPredicate ast := ctx.SemTable.AndExpressions(op.Predicates...) - predicate, err := evalengine.Translate(ast, &evalengine.Config{ - ResolveColumn: resolveFromPlan(ctx, plan, true), - Collation: ctx.SemTable.Collation, - }) - if err != nil { - return nil, err + + // this might already have been done on the operators + if predicate == nil { + predicate, err = evalengine.Translate(ast, &evalengine.Config{ + ResolveColumn: resolveFromPlan(ctx, plan, true), + Collation: ctx.SemTable.Collation, + }) + if err != nil { + return nil, err + } } return &filter{ diff --git a/go/vt/vtgate/planbuilder/operators/filter.go b/go/vt/vtgate/planbuilder/operators/filter.go index 2eee2ac28ea..2ef92c391ce 100644 --- a/go/vt/vtgate/planbuilder/operators/filter.go +++ b/go/vt/vtgate/planbuilder/operators/filter.go @@ -18,6 +18,7 @@ package operators import ( "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/rewrite" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" @@ -27,6 +28,10 @@ import ( type Filter struct { Source ops.Operator Predicates []sqlparser.Expr + + // FinalPredicate is the evalengine expression that will finally be used. + // It contains the ANDed predicates in Predicates, with ColName:s replaced by Offset:s + FinalPredicate evalengine.Expr } var _ ops.PhysicalOperator = (*Filter)(nil) diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index 3057fc0d15f..e1e1503fbcd 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -19,6 +19,7 @@ package operators import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/rewrite" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" @@ -28,7 +29,12 @@ import ( var errNotHorizonPlanned = vterrors.VT12001("query cannot be fully operator planned") +// planColumns is the process of figuring out all necessary columns. +// They can be needed because the user wants to return the value of a column, +// or because we need a column for filtering, grouping or ordering func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Operator, error) { + // We only need to do column planning to the point we hit a Route. + // Everything underneath the route is handled by the mysql planner stopAtRoute := func(operator ops.Operator) rewrite.VisitRule { _, isRoute := operator.(*Route) return rewrite.VisitRule(!isRoute) @@ -40,14 +46,20 @@ func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Opera if err != nil { if vterr, ok := err.(*vterrors.VitessError); ok && vterr.ID == "VT13001" { // we encountered a bug. let's try to back out - return nil, rewrite.SameTree, errNotHorizonPlanned + return nil, false, errNotHorizonPlanned } - return nil, rewrite.SameTree, err + return nil, false, err } return op, rewrite.NewTree, nil - case *Derived, *Filter: + case *Filter: + err := planFilter(ctx, in) + if err != nil { + return nil, false, err + } + return in, rewrite.SameTree, nil + case *Derived: // TODO we need to do column planning on these - return nil, rewrite.SameTree, errNotHorizonPlanned + return nil, false, errNotHorizonPlanned default: return in, rewrite.SameTree, nil } @@ -121,3 +133,27 @@ func planSingleRoute(rb *Route, horizon *Horizon) (ops.Operator, error) { func aeWrap(e sqlparser.Expr) *sqlparser.AliasedExpr { return &sqlparser.AliasedExpr{Expr: e} } + +func planFilter(ctx *plancontext.PlanningContext, in *Filter) error { + resolveColumn := func(col *sqlparser.ColName) (int, error) { + newSrc, offset, err := in.Source.AddColumn(ctx, aeWrap(col), true) + if err != nil { + return 0, err + } + in.Source = newSrc + return offset, nil + } + cfg := &evalengine.Config{ + ResolveType: ctx.SemTable.TypeForExpr, + Collation: ctx.SemTable.Collation, + ResolveColumn: resolveColumn, + } + + eexpr, err := evalengine.Translate(sqlparser.AndExpressions(in.Predicates...), cfg) + if err != nil { + return err + } + + in.FinalPredicate = eexpr + return nil +} diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go index 62b8d7abcf9..3b6bdc8734a 100644 --- a/go/vt/vtgate/planbuilder/operators/route_planning.go +++ b/go/vt/vtgate/planbuilder/operators/route_planning.go @@ -115,7 +115,7 @@ func optimizeDerived(ctx *plancontext.PlanningContext, op *Derived) (ops.Operato func optimizeJoin(ctx *plancontext.PlanningContext, op *Join) (ops.Operator, rewrite.TreeIdentity, error) { join, err := mergeOrJoin(ctx, op.LHS, op.RHS, sqlparser.SplitAndExpression(nil, op.Predicate), !op.LeftJoin) if err != nil { - return nil, rewrite.SameTree, err + return nil, false, err } return join, rewrite.NewTree, nil } diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index 1f3cb33bbf9..320387dee59 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -44,7 +44,7 @@ func optimizeSubQuery(ctx *plancontext.PlanningContext, op *SubQuery, ts semanti } merged, err := tryMergeSubQueryOp(ctx, outer, innerOp, newInner, preds, newSubQueryMerge(ctx, newInner), ts) if err != nil { - return nil, rewrite.SameTree, err + return nil, false, err } if merged != nil { @@ -65,13 +65,13 @@ func optimizeSubQuery(ctx *plancontext.PlanningContext, op *SubQuery, ts semanti if inner.ExtractedSubquery.OpCode == int(popcode.PulloutExists) { correlatedTree, err := createCorrelatedSubqueryOp(ctx, innerOp, outer, preds, inner.ExtractedSubquery) if err != nil { - return nil, rewrite.SameTree, err + return nil, false, err } outer = correlatedTree continue } - return nil, rewrite.SameTree, vterrors.VT12001("cross-shard correlated subquery") + return nil, false, vterrors.VT12001("cross-shard correlated subquery") } for _, tree := range unmerged { diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 377f60b6455..b2843159305 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -129,6 +129,16 @@ func (st *SemTable) CopyDependencies(from, to sqlparser.Expr) { st.Direct[to] = st.DirectDeps(from) } +// CopyDependencies copies the dependencies from one expression into the other +func (st *SemTable) Cloned(from, to sqlparser.SQLNode) { + f, fromOK := from.(sqlparser.Expr) + t, toOK := to.(sqlparser.Expr) + if !(fromOK && toOK) { + return + } + st.CopyDependencies(f, t) +} + // EmptySemTable creates a new empty SemTable func EmptySemTable() *SemTable { return &SemTable{ From eb5fd8cf21d32150b437ff922d91130f52643e3a Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 11:35:34 +0200 Subject: [PATCH 07/10] planbuilder: add simpleProjection when offsets dont line up with query Signed-off-by: Andres Taylor --- .../planbuilder/operator_transformers.go | 12 ++++ .../planbuilder/operators/horizon_planning.go | 55 +++++++++------- .../planbuilder/operators/simpleProjection.go | 63 +++++++++++++++++++ go/vt/vtgate/planbuilder/simple_projection.go | 1 - 4 files changed, 107 insertions(+), 24 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/operators/simpleProjection.go diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index e3780009c80..bde028629a5 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -59,6 +59,18 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i return transformCorrelatedSubQueryPlan(ctx, op) case *operators.Derived: return transformDerivedPlan(ctx, op) + case *operators.SimpleProjection: + src, err := transformToLogicalPlan(ctx, op.Source, false) + if err != nil { + return nil, err + } + + return &simpleProjection{ + logicalPlanCommon: newBuilderCommon(src), + eSimpleProj: &engine.SimpleProjection{ + Cols: op.Columns, + }, + }, nil case *operators.Filter: plan, err := transformToLogicalPlan(ctx, op.Source, false) if err != nil { diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index e1e1503fbcd..108ed7a1e80 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -91,38 +91,47 @@ func planHorizon(ctx *plancontext.PlanningContext, in *Horizon, isRoot bool) (op canShortcut := isRoute && sel.Having == nil && !needsOrdering _, isDerived := in.Source.(*Derived) - // if we are at the root, we have to return the columns the user asked for. in all other levels, we reuse as much as possible - canReuseCols := !isRoot - switch { case qp.NeedsAggregation() || sel.Having != nil || sel.Limit != nil || isDerived || needsOrdering || qp.NeedsDistinct(): return nil, errNotHorizonPlanned case canShortcut: return planSingleRoute(rb, in) default: - src := in.Source - for idx, e := range qp.SelectExprs { - expr, err := e.GetAliasedExpr() - if err != nil { - return nil, err - } - if !expr.As.IsEmpty() { - // we are not handling column names correct yet, so let's fail here for now - return nil, errNotHorizonPlanned - } - var offset int - src, offset, err = src.AddColumn(ctx, expr, canReuseCols) - if err != nil { - return nil, err - } - if idx != offset && isRoot { - // if we are returning something different from what the user asked for, - // we need to add an operator on top to clean up the output - return nil, errNotHorizonPlanned - } + return pushProjections(ctx, qp, in, isRoot) + } +} + +func pushProjections(ctx *plancontext.PlanningContext, qp *QueryProjection, in *Horizon, isRoot bool) (ops.Operator, error) { + // if we are at the root, we have to return the columns the user asked for. in all other levels, we reuse as much as possible + canReuseCols := !isRoot + src := in.Source + proj := newSimpleProjection(src) + needProj := false + for idx, e := range qp.SelectExprs { + expr, err := e.GetAliasedExpr() + if err != nil { + return nil, err + } + if !expr.As.IsEmpty() { + // we are not handling column names correct yet, so let's fail here for now + return nil, errNotHorizonPlanned } + var offset int + src, offset, err = src.AddColumn(ctx, expr, canReuseCols) + if err != nil { + return nil, err + } + + if offset != idx { + needProj = true + } + proj.ASTColumns = append(proj.ASTColumns, expr) + proj.Columns = append(proj.Columns, offset) + } + if !needProj { return src, nil } + return proj, nil } func planSingleRoute(rb *Route, horizon *Horizon) (ops.Operator, error) { diff --git a/go/vt/vtgate/planbuilder/operators/simpleProjection.go b/go/vt/vtgate/planbuilder/operators/simpleProjection.go new file mode 100644 index 00000000000..aeb76ae0f6d --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/simpleProjection.go @@ -0,0 +1,63 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + "golang.org/x/exp/slices" + + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" +) + +type SimpleProjection struct { + Source ops.Operator + Columns []int + ASTColumns []*sqlparser.AliasedExpr +} + +var _ ops.PhysicalOperator = (*SimpleProjection)(nil) + +func newSimpleProjection(src ops.Operator) *SimpleProjection { + return &SimpleProjection{ + Source: src, + } +} + +func (s *SimpleProjection) IPhysical() {} + +func (s *SimpleProjection) Clone(inputs []ops.Operator) ops.Operator { + return &SimpleProjection{ + Source: inputs[0], + Columns: slices.Clone(s.Columns), + ASTColumns: slices.Clone(s.ASTColumns), + } +} + +func (s *SimpleProjection) Inputs() []ops.Operator { + return []ops.Operator{s.Source} +} + +func (s *SimpleProjection) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (ops.Operator, error) { + // TODO implement me + panic("implement me") +} + +func (s *SimpleProjection) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + // TODO implement me + panic("implement me") +} diff --git a/go/vt/vtgate/planbuilder/simple_projection.go b/go/vt/vtgate/planbuilder/simple_projection.go index fb9894a89e9..c413630f386 100644 --- a/go/vt/vtgate/planbuilder/simple_projection.go +++ b/go/vt/vtgate/planbuilder/simple_projection.go @@ -35,7 +35,6 @@ var _ logicalPlan = (*simpleProjection)(nil) // a new route that keeps the subquery in the FROM // clause, because a route is more versatile than // a simpleProjection. -// this should not be used by the gen4 planner type simpleProjection struct { logicalPlanCommon resultColumns []*resultColumn From e97f05edff65edc19eccc89e406a72c808f45be4 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 15:20:24 +0200 Subject: [PATCH 08/10] planbuilder: plan columns on derived tables Signed-off-by: Andres Taylor --- go/slices2/slices.go | 8 + .../planbuilder/operators/apply_join.go | 4 + go/vt/vtgate/planbuilder/operators/derived.go | 6 + go/vt/vtgate/planbuilder/operators/filter.go | 4 + .../planbuilder/operators/horizon_planning.go | 72 +++++-- .../vtgate/planbuilder/operators/operator.go | 4 + go/vt/vtgate/planbuilder/operators/ops/op.go | 2 + go/vt/vtgate/planbuilder/operators/route.go | 4 + .../planbuilder/operators/simpleProjection.go | 7 + go/vt/vtgate/planbuilder/operators/table.go | 11 +- go/vt/vtgate/planbuilder/operators/vindex.go | 9 +- .../planbuilder/testdata/from_cases.json | 183 ++++++++---------- 12 files changed, 193 insertions(+), 121 deletions(-) diff --git a/go/slices2/slices.go b/go/slices2/slices.go index c69acb13cd5..5868c635a15 100644 --- a/go/slices2/slices.go +++ b/go/slices2/slices.go @@ -37,3 +37,11 @@ func Any[T any](s []T, fn func(T) bool) bool { } return false } + +func Map[From, To any](in []From, f func(From) To) []To { + result := make([]To, len(in)) + for i, col := range in { + result[i] = f(col) + } + return result +} diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 6f0300c569b..67165e59a02 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -157,6 +157,10 @@ func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser. return offset, nil } +func (a *ApplyJoin) GetColumns() ([]sqlparser.Expr, error) { + return a.ColumnsAST, nil +} + func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { // first check if we already are passing through this expression if reuseCol { diff --git a/go/vt/vtgate/planbuilder/operators/derived.go b/go/vt/vtgate/planbuilder/operators/derived.go index a5d467be8c2..cbaa7a2cb7a 100644 --- a/go/vt/vtgate/planbuilder/operators/derived.go +++ b/go/vt/vtgate/planbuilder/operators/derived.go @@ -19,6 +19,8 @@ package operators import ( "golang.org/x/exp/slices" + "vitess.io/vitess/go/slices2" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" "vitess.io/vitess/go/vt/sqlparser" @@ -165,6 +167,10 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Al return d, pos, nil } +func (d *Derived) GetColumns() ([]sqlparser.Expr, error) { + return slices2.Map(d.Columns, colNameToExpr), nil +} + func addToIntSlice(columnOffset []int, valToAdd int) ([]int, int) { for idx, val := range columnOffset { if val == valToAdd { diff --git a/go/vt/vtgate/planbuilder/operators/filter.go b/go/vt/vtgate/planbuilder/operators/filter.go index 2ef92c391ce..397ad71b74b 100644 --- a/go/vt/vtgate/planbuilder/operators/filter.go +++ b/go/vt/vtgate/planbuilder/operators/filter.go @@ -91,6 +91,10 @@ func (f *Filter) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Ali return f, offset, nil } +func (f *Filter) GetColumns() ([]sqlparser.Expr, error) { + return f.Source.GetColumns() +} + func (f *Filter) Compact(*plancontext.PlanningContext) (ops.Operator, rewrite.TreeIdentity, error) { if len(f.Predicates) == 0 { return f.Source, rewrite.NewTree, nil diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index 108ed7a1e80..5a853cbf916 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -44,32 +44,41 @@ func planColumns(ctx *plancontext.PlanningContext, root ops.Operator) (ops.Opera case *Horizon: op, err := planHorizon(ctx, in, in == root) if err != nil { - if vterr, ok := err.(*vterrors.VitessError); ok && vterr.ID == "VT13001" { - // we encountered a bug. let's try to back out - return nil, false, errNotHorizonPlanned - } return nil, false, err } return op, rewrite.NewTree, nil + case *Derived: + op, err := planDerived(ctx, in, in == root) + if err != nil { + return nil, false, err + } + return op, rewrite.NewTree, err case *Filter: err := planFilter(ctx, in) if err != nil { return nil, false, err } return in, rewrite.SameTree, nil - case *Derived: - // TODO we need to do column planning on these - return nil, false, errNotHorizonPlanned default: return in, rewrite.SameTree, nil } } - return rewrite.BottomUp(root, TableID, visitor, stopAtRoute) + + newOp, err := rewrite.BottomUp(root, TableID, visitor, stopAtRoute) + if err != nil { + if vterr, ok := err.(*vterrors.VitessError); ok && vterr.ID == "VT13001" { + // we encountered a bug. let's try to back out + return nil, errNotHorizonPlanned + } + return nil, err + } + + return newOp, nil } func planHorizon(ctx *plancontext.PlanningContext, in *Horizon, isRoot bool) (ops.Operator, error) { rb, isRoute := in.Source.(*Route) - if !isRoute && ctx.SemTable.NotSingleRouteErr != nil { + if isRoot && !isRoute && ctx.SemTable.NotSingleRouteErr != nil { // If we got here, we don't have a single shard plan return nil, ctx.SemTable.NotSingleRouteErr } @@ -97,14 +106,42 @@ func planHorizon(ctx *plancontext.PlanningContext, in *Horizon, isRoot bool) (op case canShortcut: return planSingleRoute(rb, in) default: - return pushProjections(ctx, qp, in, isRoot) + return pushProjections(ctx, qp, in.Source, isRoot) } } -func pushProjections(ctx *plancontext.PlanningContext, qp *QueryProjection, in *Horizon, isRoot bool) (ops.Operator, error) { +func planDerived(ctx *plancontext.PlanningContext, in *Derived, isRoot bool) (ops.Operator, error) { + rb, isRoute := in.Source.(*Route) + + sel, isSel := in.Query.(*sqlparser.Select) + if !isSel { + return nil, errNotHorizonPlanned + } + + qp, err := CreateQPFromSelect(ctx, sel) + if err != nil { + return nil, err + } + + needsOrdering := len(qp.OrderExprs) > 0 + canShortcut := isRoute && sel.Having == nil && !needsOrdering + _, isDerived := in.Source.(*Derived) + + switch { + case qp.NeedsAggregation() || sel.Having != nil || sel.Limit != nil || isDerived || needsOrdering || qp.NeedsDistinct(): + return nil, errNotHorizonPlanned + case canShortcut: + // shortcut here means we don't need to plan the derived table, we can just push it under the route + rb.Source, in.Source = in.Source, in + return rb, nil + default: + return pushProjections(ctx, qp, in.Source, isRoot) + } +} + +func pushProjections(ctx *plancontext.PlanningContext, qp *QueryProjection, src ops.Operator, isRoot bool) (ops.Operator, error) { // if we are at the root, we have to return the columns the user asked for. in all other levels, we reuse as much as possible canReuseCols := !isRoot - src := in.Source proj := newSimpleProjection(src) needProj := false for idx, e := range qp.SelectExprs { @@ -128,7 +165,16 @@ func pushProjections(ctx *plancontext.PlanningContext, qp *QueryProjection, in * proj.ASTColumns = append(proj.ASTColumns, expr) proj.Columns = append(proj.Columns, offset) } - if !needProj { + + if needProj { + return proj, nil + } + + columns, err := src.GetColumns() + if err != nil { + return nil, err + } + if len(columns) == qp.GetColumnCount() { return src, nil } return proj, nil diff --git a/go/vt/vtgate/planbuilder/operators/operator.go b/go/vt/vtgate/planbuilder/operators/operator.go index d9924779a0d..738673480ec 100644 --- a/go/vt/vtgate/planbuilder/operators/operator.go +++ b/go/vt/vtgate/planbuilder/operators/operator.go @@ -92,6 +92,10 @@ func (noColumns) AddColumn(*plancontext.PlanningContext, *sqlparser.AliasedExpr, return nil, 0, vterrors.VT13001("the noColumns operator cannot accept columns") } +func (noColumns) GetColumns() ([]sqlparser.Expr, error) { + return nil, vterrors.VT13001("the noColumns operator cannot accept columns") +} + // AddPredicate implements the Operator interface func (noPredicates) AddPredicate(*plancontext.PlanningContext, sqlparser.Expr) (ops.Operator, error) { return nil, vterrors.VT13001("the noColumns operator cannot accept predicates") diff --git a/go/vt/vtgate/planbuilder/operators/ops/op.go b/go/vt/vtgate/planbuilder/operators/ops/op.go index c3e5d649870..a22df663368 100644 --- a/go/vt/vtgate/planbuilder/operators/ops/op.go +++ b/go/vt/vtgate/planbuilder/operators/ops/op.go @@ -40,6 +40,8 @@ type ( // AddColumn tells an operator to also output an additional column specified. // The offset to the column is returned. AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (Operator, int, error) + + GetColumns() ([]sqlparser.Expr, error) } // PhysicalOperator means that this operator is ready to be turned into a logical plan diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index 2abaeea0184..bc967c82722 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -526,6 +526,10 @@ func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Alia return r, offset, nil } +func (r *Route) GetColumns() ([]sqlparser.Expr, error) { + return r.Source.GetColumns() +} + // TablesUsed returns tables used by MergedWith routes, which are not included // in Inputs() and thus not a part of the operator tree func (r *Route) TablesUsed() []string { diff --git a/go/vt/vtgate/planbuilder/operators/simpleProjection.go b/go/vt/vtgate/planbuilder/operators/simpleProjection.go index aeb76ae0f6d..f3780e3b0a6 100644 --- a/go/vt/vtgate/planbuilder/operators/simpleProjection.go +++ b/go/vt/vtgate/planbuilder/operators/simpleProjection.go @@ -19,6 +19,7 @@ package operators import ( "golang.org/x/exp/slices" + "vitess.io/vitess/go/slices2" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" @@ -61,3 +62,9 @@ func (s *SimpleProjection) AddColumn(ctx *plancontext.PlanningContext, expr *sql // TODO implement me panic("implement me") } + +func exprFromAliasedExpr(from *sqlparser.AliasedExpr) sqlparser.Expr { return from.Expr } + +func (s *SimpleProjection) GetColumns() ([]sqlparser.Expr, error) { + return slices2.Map(s.ASTColumns, exprFromAliasedExpr), nil +} diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index 1a118629dda..2cf7e5472e6 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -17,6 +17,7 @@ limitations under the License. package operators import ( + "vitess.io/vitess/go/slices2" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" @@ -34,7 +35,7 @@ type ( noInputs } ColNameColumns interface { - GetColumns() []*sqlparser.ColName + GetColNames() []*sqlparser.ColName AddCol(*sqlparser.ColName) } ) @@ -76,7 +77,11 @@ func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.Alias return to, offset, nil } -func (to *Table) GetColumns() []*sqlparser.ColName { +func (to *Table) GetColumns() ([]sqlparser.Expr, error) { + return slices2.Map(to.Columns, colNameToExpr), nil +} + +func (to *Table) GetColNames() []*sqlparser.ColName { return to.Columns } func (to *Table) AddCol(col *sqlparser.ColName) { @@ -96,7 +101,7 @@ func addColumn(op ColNameColumns, e sqlparser.Expr, reuseCol bool) (int, error) return 0, vterrors.VT13001("cannot push this expression to a table/vindex") } sqlparser.RemoveKeyspaceFromColName(col) - cols := op.GetColumns() + cols := op.GetColNames() if reuseCol { for idx, column := range cols { if col.Name.Equal(column.Name) { diff --git a/go/vt/vtgate/planbuilder/operators/vindex.go b/go/vt/vtgate/planbuilder/operators/vindex.go index 14e25857f4a..d071f617c12 100644 --- a/go/vt/vtgate/planbuilder/operators/vindex.go +++ b/go/vt/vtgate/planbuilder/operators/vindex.go @@ -17,6 +17,7 @@ limitations under the License. package operators import ( + "vitess.io/vitess/go/slices2" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" @@ -76,7 +77,13 @@ func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.Alias } -func (v *Vindex) GetColumns() []*sqlparser.ColName { +func colNameToExpr(c *sqlparser.ColName) sqlparser.Expr { return c } + +func (v *Vindex) GetColumns() ([]sqlparser.Expr, error) { + return slices2.Map(v.Columns, colNameToExpr), nil +} + +func (v *Vindex) GetColNames() []*sqlparser.ColName { return v.Columns } func (v *Vindex) AddCol(col *sqlparser.ColName) { diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 3e0aab39179..641a34bda46 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -3158,7 +3158,7 @@ "Instructions": { "OperatorType": "Join", "Variant": "Join", - "JoinColumnIndexes": "L:0", + "JoinColumnIndexes": "L:1", "JoinVars": { "t_col1": 0, "t_id": 1 @@ -3166,41 +3166,32 @@ "TableName": "`user`_user_extra_unsharded", "Inputs": [ { - "OperatorType": "SimpleProjection", - "Columns": [ - 1, - 0 - ], + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "L:0,L:1", + "TableName": "`user`_user_extra", "Inputs": [ { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "L:0,L:1", - "TableName": "`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", - "Query": "select `user`.id, `user`.col1 from `user`", - "Table": "`user`" - }, - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select 1 from user_extra where 1 != 1", - "Query": "select 1 from user_extra", - "Table": "user_extra" - } - ] + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", + "Query": "select `user`.id, `user`.col1 from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from user_extra where 1 != 1", + "Query": "select 1 from user_extra", + "Table": "user_extra" } ] }, @@ -3392,7 +3383,7 @@ "Instructions": { "OperatorType": "Join", "Variant": "Join", - "JoinColumnIndexes": "R:0", + "JoinColumnIndexes": "R:1", "TableName": "unsharded_a_`user`_user_extra", "Inputs": [ { @@ -3407,40 +3398,32 @@ "Table": "unsharded_a" }, { - "OperatorType": "SimpleProjection", - "Columns": [ - 1 - ], + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "L:0,L:1", + "TableName": "`user`_user_extra", "Inputs": [ { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "L:0,L:1", - "TableName": "`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", - "Query": "select `user`.id, `user`.col1 from `user`", - "Table": "`user`" - }, - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select 1 from user_extra where 1 != 1", - "Query": "select 1 from user_extra", - "Table": "user_extra" - } - ] + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", + "Query": "select `user`.id, `user`.col1 from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from user_extra where 1 != 1", + "Query": "select 1 from user_extra", + "Table": "user_extra" } ] } @@ -3463,7 +3446,7 @@ "Instructions": { "OperatorType": "Join", "Variant": "Join", - "JoinColumnIndexes": "R:0", + "JoinColumnIndexes": "R:1", "JoinVars": { "ua_id": 0 }, @@ -3481,44 +3464,36 @@ "Table": "unsharded_a" }, { - "OperatorType": "SimpleProjection", - "Columns": [ - 1 - ], + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "L:0,L:1", + "TableName": "`user`_user_extra", "Inputs": [ { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "L:0,L:1", - "TableName": "`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "EqualUnique", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", - "Query": "select `user`.id, `user`.col1 from `user` where `user`.id = :ua_id", - "Table": "`user`", - "Values": [ - ":ua_id" - ], - "Vindex": "user_index" - }, - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select 1 from user_extra where 1 != 1", - "Query": "select 1 from user_extra", - "Table": "user_extra" - } - ] + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.id, `user`.col1 from `user` where 1 != 1", + "Query": "select `user`.id, `user`.col1 from `user` where `user`.id = :ua_id", + "Table": "`user`", + "Values": [ + ":ua_id" + ], + "Vindex": "user_index" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from user_extra where 1 != 1", + "Query": "select 1 from user_extra", + "Table": "user_extra" } ] } From 49c29308cc703bf14da6cbfd6b8d3788a2218eba Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 29 Mar 2023 16:36:33 +0200 Subject: [PATCH 09/10] planbuilder: remove panics Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/operators/simpleProjection.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/simpleProjection.go b/go/vt/vtgate/planbuilder/operators/simpleProjection.go index f3780e3b0a6..51ee9cfec25 100644 --- a/go/vt/vtgate/planbuilder/operators/simpleProjection.go +++ b/go/vt/vtgate/planbuilder/operators/simpleProjection.go @@ -55,12 +55,12 @@ func (s *SimpleProjection) Inputs() []ops.Operator { func (s *SimpleProjection) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (ops.Operator, error) { // TODO implement me - panic("implement me") + return nil, errNotHorizonPlanned } func (s *SimpleProjection) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { // TODO implement me - panic("implement me") + return nil, 0, errNotHorizonPlanned } func exprFromAliasedExpr(from *sqlparser.AliasedExpr) sqlparser.Expr { return from.Expr } From 8266a1b121e09db1d1ee8309b25024c4940bfe8f Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 30 Mar 2023 10:37:16 +0200 Subject: [PATCH 10/10] planbuilder: minor cleanups after review feedback Signed-off-by: Andres Taylor --- .../planbuilder/operator_transformers.go | 78 ++++++++++--------- .../planbuilder/operators/apply_join.go | 10 +-- go/vt/vtgate/planbuilder/operators/derived.go | 29 +++++-- go/vt/vtgate/planbuilder/operators/horizon.go | 3 +- ...mpleProjection.go => simple_projection.go} | 3 + go/vt/vtgate/planbuilder/operators/table.go | 14 ++-- go/vt/vtgate/planbuilder/operators/vindex.go | 5 +- 7 files changed, 81 insertions(+), 61 deletions(-) rename go/vt/vtgate/planbuilder/operators/{simpleProjection.go => simple_projection.go} (89%) diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index bde028629a5..4ce7d22c857 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -60,49 +60,57 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i case *operators.Derived: return transformDerivedPlan(ctx, op) case *operators.SimpleProjection: - src, err := transformToLogicalPlan(ctx, op.Source, false) - if err != nil { - return nil, err - } - - return &simpleProjection{ - logicalPlanCommon: newBuilderCommon(src), - eSimpleProj: &engine.SimpleProjection{ - Cols: op.Columns, - }, - }, nil + return transformSimpleProjection(ctx, op) case *operators.Filter: - plan, err := transformToLogicalPlan(ctx, op.Source, false) + return transformFilter(ctx, op) + case *operators.Horizon: + return transformHorizon(ctx, op, isRoot) + } + + return nil, vterrors.VT13001(fmt.Sprintf("unknown type encountered: %T (transformToLogicalPlan)", op)) +} + +func transformFilter(ctx *plancontext.PlanningContext, op *operators.Filter) (logicalPlan, error) { + plan, err := transformToLogicalPlan(ctx, op.Source, false) + if err != nil { + return nil, err + } + + predicate := op.FinalPredicate + ast := ctx.SemTable.AndExpressions(op.Predicates...) + + // this might already have been done on the operators + if predicate == nil { + predicate, err = evalengine.Translate(ast, &evalengine.Config{ + ResolveColumn: resolveFromPlan(ctx, plan, true), + Collation: ctx.SemTable.Collation, + }) if err != nil { return nil, err } + } - predicate := op.FinalPredicate - ast := ctx.SemTable.AndExpressions(op.Predicates...) - - // this might already have been done on the operators - if predicate == nil { - predicate, err = evalengine.Translate(ast, &evalengine.Config{ - ResolveColumn: resolveFromPlan(ctx, plan, true), - Collation: ctx.SemTable.Collation, - }) - if err != nil { - return nil, err - } - } + return &filter{ + logicalPlanCommon: newBuilderCommon(plan), + efilter: &engine.Filter{ + Predicate: predicate, + ASTPredicate: ast, + }, + }, nil +} - return &filter{ - logicalPlanCommon: newBuilderCommon(plan), - efilter: &engine.Filter{ - Predicate: predicate, - ASTPredicate: ast, - }, - }, nil - case *operators.Horizon: - return transformHorizon(ctx, op, isRoot) +func transformSimpleProjection(ctx *plancontext.PlanningContext, op *operators.SimpleProjection) (logicalPlan, error) { + src, err := transformToLogicalPlan(ctx, op.Source, false) + if err != nil { + return nil, err } - return nil, vterrors.VT13001(fmt.Sprintf("unknown type encountered: %T (transformToLogicalPlan)", op)) + return &simpleProjection{ + logicalPlanCommon: newBuilderCommon(src), + eSimpleProj: &engine.SimpleProjection{ + Cols: op.Columns, + }, + }, nil } func transformHorizon(ctx *plancontext.PlanningContext, op *operators.Horizon, isRoot bool) (logicalPlan, error) { diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go index 67165e59a02..12666b73568 100644 --- a/go/vt/vtgate/planbuilder/operators/apply_join.go +++ b/go/vt/vtgate/planbuilder/operators/apply_join.go @@ -148,6 +148,7 @@ func (a *ApplyJoin) pushColLeft(ctx *plancontext.PlanningContext, e *sqlparser.A a.LHS = newLHS return offset, nil } + func (a *ApplyJoin) pushColRight(ctx *plancontext.PlanningContext, e *sqlparser.AliasedExpr) (int, error) { newRHS, offset, err := a.RHS.AddColumn(ctx, e, true) if err != nil { @@ -162,13 +163,8 @@ func (a *ApplyJoin) GetColumns() ([]sqlparser.Expr, error) { } func (a *ApplyJoin) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { - // first check if we already are passing through this expression - if reuseCol { - for i, existing := range a.ColumnsAST { - if ctx.SemTable.EqualsExpr(existing, expr.Expr) { - return a, i, nil - } - } + if offset, found := canReuseColumn(ctx, reuseCol, a.ColumnsAST, expr.Expr); found { + return a, offset, nil } lhs := TableID(a.LHS) diff --git a/go/vt/vtgate/planbuilder/operators/derived.go b/go/vt/vtgate/planbuilder/operators/derived.go index cbaa7a2cb7a..1d0e95ac329 100644 --- a/go/vt/vtgate/planbuilder/operators/derived.go +++ b/go/vt/vtgate/planbuilder/operators/derived.go @@ -140,12 +140,8 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Al return nil, 0, vterrors.VT13001("cannot push non-colname expression to a derived table") } - if reuseCol { - for offset, column := range d.Columns { - if ctx.SemTable.EqualsExpr(col, column) { - return d, offset, nil - } - } + if offset, found := canReuseColumn(ctx, reuseCol, d.Columns, col); found { + return d, offset, nil } i, err := d.findOutputColumn(col) @@ -167,6 +163,27 @@ func (d *Derived) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Al return d, pos, nil } +// canReuseColumn is generic, so it can be used with slices of different types. +// We don't care about the actual type, as long as we know it's a sqlparser.Expr +func canReuseColumn[Expr sqlparser.Expr]( + ctx *plancontext.PlanningContext, + reuseCol bool, + columns []Expr, + col sqlparser.Expr, +) (offset int, found bool) { + if !reuseCol { + return + } + + for offset, column := range columns { + if ctx.SemTable.EqualsExpr(col, column) { + return offset, true + } + } + + return +} + func (d *Derived) GetColumns() ([]sqlparser.Expr, error) { return slices2.Map(d.Columns, colNameToExpr), nil } diff --git a/go/vt/vtgate/planbuilder/operators/horizon.go b/go/vt/vtgate/planbuilder/operators/horizon.go index 74c34480caf..8be1fbb8776 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon.go +++ b/go/vt/vtgate/planbuilder/operators/horizon.go @@ -23,7 +23,8 @@ import ( ) // Horizon is an operator that allows us to postpone planning things like SELECT/GROUP BY/ORDER BY/LIMIT until later. -// It contains information about the planning we have to do after deciding how we will send the query to the tablets. // If we are able to push down the Horizon under a route, we don't have to plan these things separately and can +// It contains information about the planning we have to do after deciding how we will send the query to the tablets. +// If we are able to push down the Horizon under a route, we don't have to plan these things separately and can // just copy over the AST constructs to the query being sent to a tablet. // If we are not able to push it down, this operator needs to be split up into smaller // Project/Aggregate/Sort/Limit operations, some which can be pushed down, diff --git a/go/vt/vtgate/planbuilder/operators/simpleProjection.go b/go/vt/vtgate/planbuilder/operators/simple_projection.go similarity index 89% rename from go/vt/vtgate/planbuilder/operators/simpleProjection.go rename to go/vt/vtgate/planbuilder/operators/simple_projection.go index 51ee9cfec25..f533f7875bb 100644 --- a/go/vt/vtgate/planbuilder/operators/simpleProjection.go +++ b/go/vt/vtgate/planbuilder/operators/simple_projection.go @@ -25,6 +25,9 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" ) +// SimpleProjection is used to be selective about which columns to pass through +// All it does is to map through columns from the input +// It's used to limit the number of columns to hide result from the user that was not requested type SimpleProjection struct { Source ops.Operator Columns []int diff --git a/go/vt/vtgate/planbuilder/operators/table.go b/go/vt/vtgate/planbuilder/operators/table.go index 2cf7e5472e6..c9aad4a25dd 100644 --- a/go/vt/vtgate/planbuilder/operators/table.go +++ b/go/vt/vtgate/planbuilder/operators/table.go @@ -68,8 +68,8 @@ func (to *Table) AddPredicate(_ *plancontext.PlanningContext, expr sqlparser.Exp return newFilter(to, expr), nil } -func (to *Table) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { - offset, err := addColumn(to, expr.Expr, reuseCol) +func (to *Table) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + offset, err := addColumn(ctx, to, expr.Expr, reuseCol) if err != nil { return nil, 0, err } @@ -95,19 +95,15 @@ func (to *Table) TablesUsed() []string { return SingleQualifiedIdentifier(to.VTable.Keyspace, to.VTable.Name) } -func addColumn(op ColNameColumns, e sqlparser.Expr, reuseCol bool) (int, error) { +func addColumn(ctx *plancontext.PlanningContext, op ColNameColumns, e sqlparser.Expr, reuseCol bool) (int, error) { col, ok := e.(*sqlparser.ColName) if !ok { return 0, vterrors.VT13001("cannot push this expression to a table/vindex") } sqlparser.RemoveKeyspaceFromColName(col) cols := op.GetColNames() - if reuseCol { - for idx, column := range cols { - if col.Name.Equal(column.Name) { - return idx, nil - } - } + if offset, found := canReuseColumn(ctx, reuseCol, cols, e); found { + return offset, nil } offset := len(cols) op.AddCol(col) diff --git a/go/vt/vtgate/planbuilder/operators/vindex.go b/go/vt/vtgate/planbuilder/operators/vindex.go index d071f617c12..44045a4ed4a 100644 --- a/go/vt/vtgate/planbuilder/operators/vindex.go +++ b/go/vt/vtgate/planbuilder/operators/vindex.go @@ -67,14 +67,13 @@ func (v *Vindex) Clone([]ops.Operator) ops.Operator { var _ ops.PhysicalOperator = (*Vindex)(nil) -func (v *Vindex) AddColumn(_ *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { - offset, err := addColumn(v, expr.Expr, reuseCol) +func (v *Vindex) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseCol bool) (ops.Operator, int, error) { + offset, err := addColumn(ctx, v, expr.Expr, reuseCol) if err != nil { return nil, 0, err } return v, offset, nil - } func colNameToExpr(c *sqlparser.ColName) sqlparser.Expr { return c }