From a901e28f35358a4fe918ce3eda032fa3aab8eea9 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Fri, 8 Oct 2021 15:48:32 +0200 Subject: [PATCH 01/19] Addition of the ExtractedSubquery AST node to make easy subquery planning and its understanding Co-authored-by: Andres Taylor Signed-off-by: Florent Poinsard --- go/vt/sqlparser/ast.go | 12 ++ go/vt/sqlparser/ast_clone.go | 16 +++ go/vt/sqlparser/ast_equals.go | 28 +++++ go/vt/sqlparser/ast_format.go | 30 +++++ go/vt/sqlparser/ast_format_fast.go | 30 +++++ go/vt/sqlparser/ast_rewrite.go | 41 ++++++ go/vt/sqlparser/ast_visit.go | 22 ++++ go/vt/sqlparser/cached_size.go | 26 ++++ go/vt/vtgate/planbuilder/abstract/operator.go | 11 +- .../planbuilder/abstract/operator_test.go | 7 +- .../vtgate/planbuilder/abstract/querygraph.go | 9 ++ go/vt/vtgate/planbuilder/abstract/subquery.go | 27 +--- go/vt/vtgate/planbuilder/gen4_planner.go | 4 +- .../planbuilder/querytree_transformers.go | 55 +++----- go/vt/vtgate/planbuilder/rewrite.go | 94 ++++++-------- go/vt/vtgate/planbuilder/rewrite_test.go | 2 +- go/vt/vtgate/planbuilder/route_planning.go | 38 +++--- go/vt/vtgate/planbuilder/routetree.go | 117 ++++++++++++------ go/vt/vtgate/planbuilder/subquerytree.go | 11 +- .../planbuilder/testdata/filter_cases.txt | 2 +- go/vt/vtgate/semantics/binder.go | 18 +-- go/vt/vtgate/semantics/scoper.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 19 +-- go/vt/vtgate/semantics/tabletset.go | 31 +++++ go/vt/vtgate/semantics/tabletset_test.go | 8 ++ 25 files changed, 436 insertions(+), 224 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 46b79313377..38e713e8cb1 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -1963,6 +1963,17 @@ type ( Name ColIdent Fsp Expr // fractional seconds precision, integer from 0 to 6 } + + // ExtractedSubquery is a subquery that has been extracted from the original AST + // This is a struct that the parser will never produce - it's written and read by the gen4 planner + ExtractedSubquery struct { + Original Expr + ArgName string + HasValuesArg string + OpCode int // this should really be engine.PulloutOpCode, but we cannot depend on engine :( + Subquery SelectStatement + OtherSide Expr // represents the side of the comparison, this field will be nil if Original is not a comparison + } ) // iExpr ensures that only expressions nodes can be assigned to a Expr @@ -1997,6 +2008,7 @@ func (*ConvertUsingExpr) iExpr() {} func (*MatchExpr) iExpr() {} func (*GroupConcatExpr) iExpr() {} func (*Default) iExpr() {} +func (*ExtractedSubquery) iExpr() {} // Exprs represents a list of value expressions. // It's not a valid expression because it's not parenthesized. diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go index 5cec7e2ef6d..375e0116132 100644 --- a/go/vt/sqlparser/ast_clone.go +++ b/go/vt/sqlparser/ast_clone.go @@ -129,6 +129,8 @@ func CloneSQLNode(in SQLNode) SQLNode { return CloneRefOfExplainTab(in) case Exprs: return CloneExprs(in) + case *ExtractedSubquery: + return CloneRefOfExtractedSubquery(in) case *Flush: return CloneRefOfFlush(in) case *Force: @@ -853,6 +855,18 @@ func CloneExprs(n Exprs) Exprs { return res } +// CloneRefOfExtractedSubquery creates a deep clone of the input. +func CloneRefOfExtractedSubquery(n *ExtractedSubquery) *ExtractedSubquery { + if n == nil { + return nil + } + out := *n + out.Original = CloneExpr(n.Original) + out.Subquery = CloneSelectStatement(n.Subquery) + out.OtherSide = CloneExpr(n.OtherSide) + return &out +} + // CloneRefOfFlush creates a deep clone of the input. func CloneRefOfFlush(n *Flush) *Flush { if n == nil { @@ -2023,6 +2037,8 @@ func CloneExpr(in Expr) Expr { return CloneRefOfDefault(in) case *ExistsExpr: return CloneRefOfExistsExpr(in) + case *ExtractedSubquery: + return CloneRefOfExtractedSubquery(in) case *FuncExpr: return CloneRefOfFuncExpr(in) case *GroupConcatExpr: diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index 1b75186917e..04a1e6cc654 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -344,6 +344,12 @@ func EqualsSQLNode(inA, inB SQLNode) bool { return false } return EqualsExprs(a, b) + case *ExtractedSubquery: + b, ok := inB.(*ExtractedSubquery) + if !ok { + return false + } + return EqualsRefOfExtractedSubquery(a, b) case *Flush: b, ok := inB.(*Flush) if !ok { @@ -1549,6 +1555,22 @@ func EqualsExprs(a, b Exprs) bool { return true } +// EqualsRefOfExtractedSubquery does deep equals between the two objects. +func EqualsRefOfExtractedSubquery(a, b *ExtractedSubquery) bool { + if a == b { + return true + } + if a == nil || b == nil { + return false + } + return a.ArgName == b.ArgName && + a.HasValuesArg == b.HasValuesArg && + a.OpCode == b.OpCode && + EqualsExpr(a.Original, b.Original) && + EqualsSelectStatement(a.Subquery, b.Subquery) && + EqualsExpr(a.OtherSide, b.OtherSide) +} + // EqualsRefOfFlush does deep equals between the two objects. func EqualsRefOfFlush(a, b *Flush) bool { if a == b { @@ -3109,6 +3131,12 @@ func EqualsExpr(inA, inB Expr) bool { return false } return EqualsRefOfExistsExpr(a, b) + case *ExtractedSubquery: + b, ok := inB.(*ExtractedSubquery) + if !ok { + return false + } + return EqualsRefOfExtractedSubquery(a, b) case *FuncExpr: b, ok := inB.(*FuncExpr) if !ok { diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index 4abce0075c7..fb85803211f 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -1720,3 +1720,33 @@ func (node *RenameTable) Format(buf *TrackedBuffer) { prefix = ", " } } + +func (node *ExtractedSubquery) Format(buf *TrackedBuffer) { + switch original := node.Original.(type) { + case *ExistsExpr: + buf.astPrintf(node, "%v", NewArgument(node.ArgName)) + case *ComparisonExpr: + // other_side = :__sq + cmp := &ComparisonExpr{ + Left: node.OtherSide, + Right: NewArgument(node.ArgName), + Operator: original.Operator, + } + var expr Expr = cmp + switch original.Operator { + case InOp: + // :__sq_has_values = 1 and other_side in ::__sq + cmp.Right = NewListArg(node.ArgName) + hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("1"), Operator: EqualOp} + expr = AndExpressions(hasValue, cmp) + case NotInOp: + // :__sq_has_values = 0 or other_side not in ::__sq + cmp.Right = NewListArg(node.ArgName) + hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("0"), Operator: EqualOp} + expr = OrExpressions(hasValue, cmp) + } + buf.astPrintf(node, "%v", expr) + case *Subquery: + buf.astPrintf(node, "%v", NewArgument(node.ArgName)) + } +} diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index ca1802220cb..a7febaceca5 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -2250,3 +2250,33 @@ func (node *RenameTable) formatFast(buf *TrackedBuffer) { prefix = ", " } } + +func (node *ExtractedSubquery) formatFast(buf *TrackedBuffer) { + switch original := node.Original.(type) { + case *ExistsExpr: + buf.printExpr(node, NewArgument(node.ArgName), true) + case *ComparisonExpr: + // other_side = :__sq + cmp := &ComparisonExpr{ + Left: node.OtherSide, + Right: NewArgument(node.ArgName), + Operator: original.Operator, + } + var expr Expr = cmp + switch original.Operator { + case InOp: + // :__sq_has_values = 1 and other_side in ::__sq + cmp.Right = NewListArg(node.ArgName) + hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("1"), Operator: EqualOp} + expr = AndExpressions(hasValue, cmp) + case NotInOp: + // :__sq_has_values = 0 or other_side not in ::__sq + cmp.Right = NewListArg(node.ArgName) + hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("0"), Operator: EqualOp} + expr = OrExpressions(hasValue, cmp) + } + buf.printExpr(node, expr, true) + case *Subquery: + buf.printExpr(node, NewArgument(node.ArgName), true) + } +} diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go index 6da577e73cb..f26d237f4ac 100644 --- a/go/vt/sqlparser/ast_rewrite.go +++ b/go/vt/sqlparser/ast_rewrite.go @@ -128,6 +128,8 @@ func (a *application) rewriteSQLNode(parent SQLNode, node SQLNode, replacer repl return a.rewriteRefOfExplainTab(parent, node, replacer) case Exprs: return a.rewriteExprs(parent, node, replacer) + case *ExtractedSubquery: + return a.rewriteRefOfExtractedSubquery(parent, node, replacer) case *Flush: return a.rewriteRefOfFlush(parent, node, replacer) case *Force: @@ -1887,6 +1889,43 @@ func (a *application) rewriteExprs(parent SQLNode, node Exprs, replacer replacer } return true } +func (a *application) rewriteRefOfExtractedSubquery(parent SQLNode, node *ExtractedSubquery, replacer replacerFunc) bool { + if node == nil { + return true + } + if a.pre != nil { + a.cur.replacer = replacer + a.cur.parent = parent + a.cur.node = node + if !a.pre(&a.cur) { + return true + } + } + if !a.rewriteExpr(node, node.Original, func(newNode, parent SQLNode) { + parent.(*ExtractedSubquery).Original = newNode.(Expr) + }) { + return false + } + if !a.rewriteSelectStatement(node, node.Subquery, func(newNode, parent SQLNode) { + parent.(*ExtractedSubquery).Subquery = newNode.(SelectStatement) + }) { + return false + } + if !a.rewriteExpr(node, node.OtherSide, func(newNode, parent SQLNode) { + parent.(*ExtractedSubquery).OtherSide = newNode.(Expr) + }) { + return false + } + if a.post != nil { + a.cur.replacer = replacer + a.cur.parent = parent + a.cur.node = node + if !a.post(&a.cur) { + return false + } + } + return true +} func (a *application) rewriteRefOfFlush(parent SQLNode, node *Flush, replacer replacerFunc) bool { if node == nil { return true @@ -4973,6 +5012,8 @@ func (a *application) rewriteExpr(parent SQLNode, node Expr, replacer replacerFu return a.rewriteRefOfDefault(parent, node, replacer) case *ExistsExpr: return a.rewriteRefOfExistsExpr(parent, node, replacer) + case *ExtractedSubquery: + return a.rewriteRefOfExtractedSubquery(parent, node, replacer) case *FuncExpr: return a.rewriteRefOfFuncExpr(parent, node, replacer) case *GroupConcatExpr: diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go index e4790f3df0c..2a110e53fff 100644 --- a/go/vt/sqlparser/ast_visit.go +++ b/go/vt/sqlparser/ast_visit.go @@ -128,6 +128,8 @@ func VisitSQLNode(in SQLNode, f Visit) error { return VisitRefOfExplainTab(in, f) case Exprs: return VisitExprs(in, f) + case *ExtractedSubquery: + return VisitRefOfExtractedSubquery(in, f) case *Flush: return VisitRefOfFlush(in, f) case *Force: @@ -1027,6 +1029,24 @@ func VisitExprs(in Exprs, f Visit) error { } return nil } +func VisitRefOfExtractedSubquery(in *ExtractedSubquery, f Visit) error { + if in == nil { + return nil + } + if cont, err := f(in); err != nil || !cont { + return err + } + if err := VisitExpr(in.Original, f); err != nil { + return err + } + if err := VisitSelectStatement(in.Subquery, f); err != nil { + return err + } + if err := VisitExpr(in.OtherSide, f); err != nil { + return err + } + return nil +} func VisitRefOfFlush(in *Flush, f Visit) error { if in == nil { return nil @@ -2487,6 +2507,8 @@ func VisitExpr(in Expr, f Visit) error { return VisitRefOfDefault(in, f) case *ExistsExpr: return VisitRefOfExistsExpr(in, f) + case *ExtractedSubquery: + return VisitRefOfExtractedSubquery(in, f) case *FuncExpr: return VisitRefOfFuncExpr(in, f) case *GroupConcatExpr: diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go index 287c367839c..ef25f7d2e11 100644 --- a/go/vt/sqlparser/cached_size.go +++ b/go/vt/sqlparser/cached_size.go @@ -928,6 +928,32 @@ func (cached *ExplainTab) CachedSize(alloc bool) int64 { size += hack.RuntimeAllocSize(int64(len(cached.Wild))) return size } +func (cached *ExtractedSubquery) CachedSize(alloc bool) int64 { + if cached == nil { + return int64(0) + } + size := int64(0) + if alloc { + size += int64(96) + } + // field Original vitess.io/vitess/go/vt/sqlparser.Expr + if cc, ok := cached.Original.(cachedObject); ok { + size += cc.CachedSize(true) + } + // field ArgName string + size += hack.RuntimeAllocSize(int64(len(cached.ArgName))) + // field HasValuesArg string + size += hack.RuntimeAllocSize(int64(len(cached.HasValuesArg))) + // field Subquery vitess.io/vitess/go/vt/sqlparser.SelectStatement + if cc, ok := cached.Subquery.(cachedObject); ok { + size += cc.CachedSize(true) + } + // field OtherSide vitess.io/vitess/go/vt/sqlparser.Expr + if cc, ok := cached.OtherSide.(cachedObject); ok { + size += cc.CachedSize(true) + } + return size +} func (cached *Flush) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index f491ac11214..bb01f7bf92b 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -201,7 +201,7 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl if len(semTable.SubqueryMap[sel]) > 0 { resultantOp = &SubQuery{} for _, sq := range semTable.SubqueryMap[sel] { - subquerySelectStatement, isSel := sq.SubQuery.Select.(*sqlparser.Select) + subquerySelectStatement, isSel := sq.Subquery.(*sqlparser.Select) if !isSel { return nil, semantics.Gen4NotSupportedF("UNION in subquery") } @@ -210,13 +210,8 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl return nil, err } resultantOp.Inner = append(resultantOp.Inner, &SubQueryInner{ - SelectStatement: subquerySelectStatement, - Inner: opInner, - Type: sq.OpCode, - ArgName: sq.ArgName, - HasValues: sq.HasValues, - ExprsNeedReplace: sq.ExprsNeedReplace, - ReplaceBy: sq.ReplaceBy, + Inner: opInner, + ExtractedSubquery: sq, }) } } diff --git a/go/vt/vtgate/planbuilder/abstract/operator_test.go b/go/vt/vtgate/planbuilder/abstract/operator_test.go index 9e1a7c90ddb..fc6edfe3d4a 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator_test.go +++ b/go/vt/vtgate/planbuilder/abstract/operator_test.go @@ -24,6 +24,7 @@ import ( "sort" "strings" "testing" + "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -130,9 +131,9 @@ func testString(op Operator) string { case *SubQuery: var inners []string for _, sqOp := range op.Inner { - subquery := fmt.Sprintf("{\n\tType: %s", sqOp.Type.String()) - if sqOp.ArgName != "" { - subquery += fmt.Sprintf("\n\tArgName: %s", sqOp.ArgName) + subquery := fmt.Sprintf("{\n\tType: %s", engine.PulloutOpcode(sqOp.ExtractedSubquery.OpCode).String()) + if sqOp.ExtractedSubquery.ArgName != "" { + subquery += fmt.Sprintf("\n\tArgName: %s", sqOp.ExtractedSubquery.ArgName) } subquery += fmt.Sprintf("\n\tQuery: %s\n}", indent(testString(sqOp.Inner))) subquery = indent(subquery) diff --git a/go/vt/vtgate/planbuilder/abstract/querygraph.go b/go/vt/vtgate/planbuilder/abstract/querygraph.go index 7f5db077309..6e03407e32d 100644 --- a/go/vt/vtgate/planbuilder/abstract/querygraph.go +++ b/go/vt/vtgate/planbuilder/abstract/querygraph.go @@ -133,6 +133,15 @@ func (qg *QueryGraph) addJoinPredicates(ts semantics.TableSet, expr sqlparser.Ex func (qg *QueryGraph) collectPredicate(predicate sqlparser.Expr, semTable *semantics.SemTable) error { deps := semTable.RecursiveDeps(predicate) + extracted, ok := predicate.(*sqlparser.ExtractedSubquery) + if ok { + // we don't want to look at the subquery dependencies + if extracted.OtherSide == nil { + deps = semantics.SingleTableSet(0) + } else if semantics.ValidAsMapKey(extracted.OtherSide) { + deps = semTable.RecursiveDeps(extracted.OtherSide) + } + } switch deps.NumberOfTables() { case 0: qg.addNoDepsPredicate(predicate) diff --git a/go/vt/vtgate/planbuilder/abstract/subquery.go b/go/vt/vtgate/planbuilder/abstract/subquery.go index fa92f24a3da..b3642fb77c9 100644 --- a/go/vt/vtgate/planbuilder/abstract/subquery.go +++ b/go/vt/vtgate/planbuilder/abstract/subquery.go @@ -20,7 +20,6 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -39,30 +38,8 @@ type SubQueryInner struct { // of type Concatenate since we have a Union. Inner Operator - // Type represents the type of the subquery (value, in, not in, exists) - Type engine.PulloutOpcode - - // SelectStatement is the inner's select - SelectStatement *sqlparser.Select - - // ArgName is the substitution argument string for the subquery. - // Subquery argument name looks like: `__sq1`, with `1` being an - // unique identifier. This is used when we wish to replace the - // subquery by an argument for PullOut subqueries. - ArgName string - - // HasValues is a string of form `__sq_has_values1` with `1` being - // a unique identifier that matches the one used in ArgName. - // We use `__sq_has_values` for in and not in subqueries. - HasValues string - - // ExprsNeedReplace is a slice of all the expressions that were - // introduced by the rewrite of the subquery and that potentially - // need to be re-replace if we can merge the subquery into a route. - // An expression that contains at least all of ExprsNeedReplace will - // be replaced by the expression in ReplaceBy. - ExprsNeedReplace []sqlparser.Expr - ReplaceBy sqlparser.Expr + // ExtractedSubquery contains all information we need about this subquery + ExtractedSubquery *sqlparser.ExtractedSubquery } // TableID implements the Operator interface diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 9913b15d825..cd9e73893f4 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -181,8 +181,6 @@ func newPlanningContext(reservedVars *sqlparser.ReservedVars, semTable *semantic reservedVars: reservedVars, semTable: semTable, vschema: vschema, - argToReplaceBySelect: map[string]*sqlparser.Select{}, - exprToReplaceBySqExpr: map[sqlparser.Expr]sqlparser.Expr{}, } return ctx } @@ -217,7 +215,7 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat sel: node, } - replaceSubQuery(ctx.exprToReplaceBySqExpr, node) + replaceSubQuery(ctx.reinsertSubQ, node) var err error plan, err = hp.planHorizon(ctx, plan) if err != nil { diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index 570af070f99..a30922627cf 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -93,12 +93,12 @@ func transformSubqueryTree(ctx *planningContext, n *subqueryTree) (logicalPlan, if err != nil { return nil, err } - innerPlan, err = planHorizon(ctx, innerPlan, n.subquery) + innerPlan, err = planHorizon(ctx, innerPlan, n.extracted.Subquery) if err != nil { return nil, err } - plan := newPulloutSubquery(n.opcode, n.argName, n.hasValues, innerPlan) + plan := newPulloutSubquery(engine.PulloutOpcode(n.extracted.OpCode), n.extracted.ArgName, n.extracted.HasValuesArg, innerPlan) outerPlan, err := transformToLogicalPlan(ctx, n.outer) if err != nil { return nil, err @@ -381,7 +381,7 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { Comments: ctx.semTable.Comments, } - replaceSubQuery(ctx.exprToReplaceBySqExpr, sel) + replaceSubQuery(ctx.reinsertSubQ, sel) // TODO clean up when gen4 is the only planner var condition sqlparser.Expr @@ -496,54 +496,27 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { } type subQReplacer struct { - exprToReplaceBySqExpr map[sqlparser.Expr]sqlparser.Expr + exprToReplaceBySqExpr []*sqlparser.ExtractedSubquery replaced bool } func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { - var exprs []sqlparser.Expr - switch node := cursor.Node().(type) { - case *sqlparser.AndExpr: - exprs = sqlparser.SplitAndExpression(nil, node) - case *sqlparser.OrExpr: - exprs = sqlparser.SplitOrExpression(nil, node) - case sqlparser.Argument: - exprs = append(exprs, node) - case sqlparser.ListArg: - exprs = append(exprs, node) - case *sqlparser.ExistsExpr: - exprs = append(exprs, node) - default: + ext, ok := cursor.Node().(*sqlparser.ExtractedSubquery) + if !ok { return true } - - var replaceBy sqlparser.Expr - var remainder sqlparser.Expr - for _, expr := range exprs { - found := false - for sqExprToReplace, replaceByExpr := range sqr.exprToReplaceBySqExpr { - if sqlparser.EqualsExpr(expr, sqExprToReplace) { - allReplaceByExprs := sqlparser.SplitAndExpression(nil, replaceBy) - allReplaceByExprs = append(allReplaceByExprs, replaceByExpr) - replaceBy = sqlparser.AndExpressions(allReplaceByExprs...) - found = true - break - } + for _, replaceByExpr := range sqr.exprToReplaceBySqExpr { + // we are comparing the ArgNames in case the expressions have been cloned + if ext.ArgName == replaceByExpr.ArgName { + cursor.Replace(ext.Original) + sqr.replaced = true + return false } - if !found { - remainder = sqlparser.AndExpressions(remainder, expr) - } - } - if replaceBy == nil { - return true } - newNode := sqlparser.AndExpressions(remainder, replaceBy) - cursor.Replace(newNode) - sqr.replaced = true - return false + return true } -func replaceSubQuery(exprToReplaceBySqExpr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { +func replaceSubQuery(exprToReplaceBySqExpr []*sqlparser.ExtractedSubquery, sel *sqlparser.Select) { if len(exprToReplaceBySqExpr) > 0 { sqr := &subQReplacer{exprToReplaceBySqExpr: exprToReplaceBySqExpr} sqlparser.Rewrite(sel, sqr.replacer, nil) diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index a07187fcaab..be8081096c5 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -17,7 +17,9 @@ limitations under the License. package planbuilder import ( + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -26,6 +28,7 @@ type rewriter struct { semTable *semantics.SemTable reservedVars *sqlparser.ReservedVars isInSubquery int + err error } func queryRewrite(semTable *semantics.SemTable, reservedVars *sqlparser.ReservedVars, statement sqlparser.SelectStatement) error { @@ -42,12 +45,16 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { case *sqlparser.Select: rewriteHavingClause(node) case *sqlparser.ComparisonExpr: - rewriteInSubquery(cursor, r, node) + err := rewriteInSubquery(cursor, r, node) + if err != nil { + r.err = err + } case *sqlparser.ExistsExpr: - return r.rewriteExistsSubquery(cursor, node) - case *sqlparser.Subquery: - r.isInSubquery++ - rewriteSubquery(cursor, r, node) + err := r.rewriteExistsSubquery(cursor, node) + if err != nil { + r.err = err + } + return false case *sqlparser.AliasedTableExpr: // rewrite names of the routed tables for the subquery // We only need to do this for non-derived tables and if they are in a subquery @@ -91,55 +98,35 @@ func (r *rewriter) rewriteUp(cursor *sqlparser.Cursor) bool { case *sqlparser.Subquery: r.isInSubquery-- } - return true + return r.err == nil } -func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.ComparisonExpr) { - if node.Operator != sqlparser.InOp && node.Operator != sqlparser.NotInOp { - return +func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.ComparisonExpr) error { + if node.Operator != sqlparser.InOp && + node.Operator != sqlparser.NotInOp && + node.Operator != sqlparser.EqualOp && + node.Operator != sqlparser.NotEqualOp { + return nil } - subq, exp := filterSubqueryAndCompareExpr(node) + subq, exp := getSubqueryAndOtherSide(node) if subq == nil || exp == nil { - return + return nil } semTableSQ, found := r.semTable.SubqueryRef[subq] if !found { - // should never happen - return + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName - semTableSQ.HasValues = hasValuesArg - semTableSQ.ReplaceBy = node - newSubQExpr := &sqlparser.ComparisonExpr{ - Operator: node.Operator, - Left: exp, - Right: sqlparser.NewListArg(argName), - } - - hasValuesExpr := &sqlparser.ComparisonExpr{ - Operator: sqlparser.EqualOp, - Left: sqlparser.NewArgument(hasValuesArg), - } - switch node.Operator { - case sqlparser.InOp: - hasValuesExpr.Right = sqlparser.NewIntLiteral("1") - cursor.Replace(sqlparser.AndExpressions(hasValuesExpr, newSubQExpr)) - case sqlparser.NotInOp: - hasValuesExpr.Right = sqlparser.NewIntLiteral("0") - cursor.Replace(sqlparser.OrExpressions(hasValuesExpr, newSubQExpr)) - } - semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, hasValuesExpr, newSubQExpr) - - // setting the dependencies of the new has_value expression to the subquery's dependencies so - // later on, we know has_values depends on the same tables as the subquery - r.semTable.Recursive[hasValuesExpr] = r.semTable.RecursiveDeps(newSubQExpr) - r.semTable.Direct[hasValuesExpr] = r.semTable.DirectDeps(newSubQExpr) + semTableSQ.HasValuesArg = hasValuesArg + semTableSQ.OtherSide = exp + cursor.Replace(semTableSQ) + return nil } -func filterSubqueryAndCompareExpr(node *sqlparser.ComparisonExpr) (*sqlparser.Subquery, sqlparser.Expr) { +func getSubqueryAndOtherSide(node *sqlparser.ComparisonExpr) (*sqlparser.Subquery, sqlparser.Expr) { var subq *sqlparser.Subquery var exp sqlparser.Expr if lSubq, lIsSubq := node.Left.(*sqlparser.Subquery); lIsSubq { @@ -152,34 +139,31 @@ func filterSubqueryAndCompareExpr(node *sqlparser.ComparisonExpr) (*sqlparser.Su return subq, exp } -func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) { +func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) error { semTableSQ, found := r.semTable.SubqueryRef[node] - if !found || semTableSQ.OpCode != engine.PulloutValue { - return + if !found { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") + } + if engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { + return nil } argName := r.reservedVars.ReserveSubQuery() - arg := sqlparser.NewArgument(argName) semTableSQ.ArgName = argName - semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, arg) - semTableSQ.ReplaceBy = node - cursor.Replace(arg) + cursor.Replace(semTableSQ) + return nil } -func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlparser.ExistsExpr) bool { +func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlparser.ExistsExpr) error { semTableSQ, found := r.semTable.SubqueryRef[node.Subquery] if !found { - // should never happen - return false + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } argName := r.reservedVars.ReserveHasValuesSubQuery() - arg := sqlparser.NewArgument(argName) semTableSQ.ArgName = argName - semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, arg) - semTableSQ.ReplaceBy = node - cursor.Replace(arg) - return false + cursor.Replace(semTableSQ) + return nil } func rewriteHavingClause(node *sqlparser.Select) { diff --git a/go/vt/vtgate/planbuilder/rewrite_test.go b/go/vt/vtgate/planbuilder/rewrite_test.go index cda6bf66f0b..a239a36bdd6 100644 --- a/go/vt/vtgate/planbuilder/rewrite_test.go +++ b/go/vt/vtgate/planbuilder/rewrite_test.go @@ -140,7 +140,7 @@ func TestHavingRewrite(t *testing.T) { assert.Equal(t, len(tcase.sqs), len(squeries), "number of subqueries not matched") } for _, sq := range squeries { - assert.Equal(t, tcase.sqs[sq.ArgName], sqlparser.String(sq.SubQuery.Select)) + assert.Equal(t, tcase.sqs[sq.ArgName], sqlparser.String(sq.Subquery)) } }) } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 00efccc5809..1ed7fe4b659 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -39,14 +39,21 @@ type planningContext struct { semTable *semantics.SemTable vschema ContextVSchema // these help in replacing the argNames with the subquery - argToReplaceBySelect map[string]*sqlparser.Select - // these help in replacing the argument's expressions by the original expression - exprToReplaceBySqExpr map[sqlparser.Expr]sqlparser.Expr + reinsertSubQ []*sqlparser.ExtractedSubquery } -func (c planningContext) isSubQueryToReplace(name string) bool { - _, found := c.argToReplaceBySelect[name] - return found +func (c planningContext) isSubQueryToReplace(e sqlparser.Expr) bool { + ext, ok := e.(*sqlparser.ExtractedSubquery) + if !ok { + return false + } + for _, subq := range c.reinsertSubQ{ + if subq == ext { + return true + } + } + + return false } func optimizeQuery(ctx *planningContext, opTree abstract.Operator) (queryTree, error) { @@ -152,11 +159,8 @@ func optimizeSubQuery(ctx *planningContext, op *abstract.SubQuery) (queryTree, e return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: cross-shard correlated subquery") } unmerged = append(unmerged, &subqueryTree{ - subquery: inner.SelectStatement, + extracted: inner.ExtractedSubquery, inner: treeInner, - opcode: inner.Type, - argName: inner.ArgName, - hasValues: inner.HasValues, }) } else { outerTree = merged @@ -237,16 +241,16 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre // other side is RHS if the subquery is in the LHS, otherwise it is LHS var rewriteError error // go over the entire where expression in the subquery - sqlparser.Rewrite(subQueryInner.SelectStatement, func(cursor *sqlparser.Cursor) bool { + sqlparser.Rewrite(subQueryInner.ExtractedSubquery.Original, func(cursor *sqlparser.Cursor) bool { sqlNode := cursor.Node() switch node := sqlNode.(type) { case *sqlparser.ColName: - // check weather the column name belongs to the other side of the join tree + // check whether the column name belongs to the other side of the join tree if ctx.semTable.DirectDeps(node).IsSolvedBy(otherTree.tableID()) { // get the bindVariable for that column name and replace it in the subquery bindVar := node.CompliantName() cursor.Replace(sqlparser.NewArgument(bindVar)) - // check wether the bindVariable already exists in the joinVars of the other tree + // check whether the bindVariable already exists in the joinVars of the other tree _, alreadyExists := outerTree.vars[bindVar] if alreadyExists { return false @@ -270,10 +274,8 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre } func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { - ctx.argToReplaceBySelect[subq.ArgName] = subq.SelectStatement - for _, expr := range subq.ExprsNeedReplace { - ctx.exprToReplaceBySqExpr[expr] = subq.ReplaceBy - } + ctx.reinsertSubQ = append(ctx.reinsertSubQ, subq.ExtractedSubquery) + // go over the subquery and add its tables to the one's solved by the route it is merged with // this is needed to so that later when we try to push projections, we get the correct // solved tableID from the route, since it also includes the tables from the subquery after merging @@ -283,7 +285,7 @@ func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, sub outer.solved.MergeInPlace(ctx.semTable.TableSetFor(n)) } return true, nil - }, subq.SelectStatement) + }, subq.ExtractedSubquery.Subquery) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index 29bcb4d1fac..ec9e8792880 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -191,43 +191,22 @@ func (rp *routeTree) searchForNewVindexes(ctx *planningContext, predicates []sql newVindexFound := false for _, filter := range predicates { switch node := filter.(type) { - case *sqlparser.ComparisonExpr: - if sqlparser.IsNull(node.Left) || sqlparser.IsNull(node.Right) { - // we are looking at ANDed predicates in the WHERE clause. - // since we know that nothing returns true when compared to NULL, - // so we can safely bail out here - rp.routeOpCode = engine.SelectNone - return false, nil + case *sqlparser.ExtractedSubquery: + comp, ok := node.Original.(*sqlparser.ComparisonExpr) + if !ok { + break } - - switch node.Operator { - case sqlparser.EqualOp: - found, err := rp.planEqualOp(ctx, node) - if err != nil { - return false, err - } - newVindexFound = newVindexFound || found - case sqlparser.InOp: - if rp.isImpossibleIN(node) { - return false, nil - } - found, err := rp.planInOp(ctx, node) - if err != nil { - return false, err - } - newVindexFound = newVindexFound || found - case sqlparser.NotInOp: - // NOT IN is always a scatter, except when we can be sure it would return nothing - if rp.isImpossibleNotIN(node) { - return false, nil - } - case sqlparser.LikeOp: - found, err := rp.planLikeOp(ctx, node) - if err != nil { - return false, err - } - newVindexFound = newVindexFound || found + found, err := rp.planComparison(ctx, comp) + if err != nil { + return false, err + } + newVindexFound = newVindexFound || found + case *sqlparser.ComparisonExpr: + found, err := rp.planComparison(ctx, node) + if err != nil { + return false, err } + newVindexFound = newVindexFound || found case *sqlparser.IsExpr: found, err := rp.planIsExpr(ctx, node) if err != nil { @@ -239,6 +218,46 @@ func (rp *routeTree) searchForNewVindexes(ctx *planningContext, predicates []sql return newVindexFound, nil } +func (rp *routeTree) planComparison(ctx *planningContext, node *sqlparser.ComparisonExpr) (bool, error) { + if sqlparser.IsNull(node.Left) || sqlparser.IsNull(node.Right) { + // we are looking at ANDed predicates in the WHERE clause. + // since we know that nothing returns true when compared to NULL, + // so we can safely bail out here + rp.routeOpCode = engine.SelectNone + return false, nil + } + + switch node.Operator { + case sqlparser.EqualOp: + found, err := rp.planEqualOp(ctx, node) + if err != nil { + return false, err + } + return found, nil + case sqlparser.InOp: + if rp.isImpossibleIN(node) { + return false, nil + } + found, err := rp.planInOp(ctx, node) + if err != nil { + return false, err + } + return found, nil + case sqlparser.NotInOp: + // NOT IN is always a scatter, except when we can be sure it would return nothing + if rp.isImpossibleNotIN(node) { + return false, nil + } + case sqlparser.LikeOp: + found, err := rp.planLikeOp(ctx, node) + if err != nil { + return false, err + } + return found, nil + } + return false, nil +} + func (rp *routeTree) isImpossibleIN(node *sqlparser.ComparisonExpr) bool { switch nodeR := node.Right.(type) { case sqlparser.ValTuple: @@ -414,11 +433,24 @@ func (rp *routeTree) planIsExpr(ctx *planningContext, node *sqlparser.IsExpr) (b // Otherwise, the method will try to apply makePlanValue for any equality the sqlparser.Expr n has. // The first PlanValue that is successfully produced will be returned. func (rp *routeTree) makePlanValue(ctx *planningContext, n sqlparser.Expr) (*sqltypes.PlanValue, error) { - if ctx.isSubQueryToReplace(argumentName(n)) { + if ctx.isSubQueryToReplace(n) { return nil, nil } for _, expr := range ctx.semTable.GetExprAndEqualities(n) { + if subq, isSubq := expr.(*sqlparser.Subquery); isSubq { + for subquery, extractedSubquery := range ctx.semTable.SubqueryRef { + if sqlparser.EqualsRefOfSubquery(subquery, subq) { + switch engine.PulloutOpcode(extractedSubquery.OpCode) { + case engine.PulloutIn, engine.PulloutNotIn: + expr = sqlparser.NewListArg(extractedSubquery.ArgName) + case engine.PulloutValue, engine.PulloutExists: + expr = sqlparser.NewArgument(extractedSubquery.ArgName) + } + break + } + } + } pv, err := makePlanValue(expr) if err != nil { return nil, err @@ -451,6 +483,19 @@ func allNotNil(s []sqlparser.Expr) bool { return true } +// func (rp *routeTree) findMatchingColumnVindex(ctx *planningContext, column *sqlparser.ColName) bool { +// for _, pred := range rp.vindexPreds { +// if !ctx.semTable.DirectDeps(column).IsSolvedBy(pred.tableID) { +// continue +// } +// for _, vindexCol := range pred.colVindex.Columns { +// if column.Name.Equal(vindexCol) { +// +// } +// } +// } +// } + func (rp *routeTree) haveMatchingVindex( ctx *planningContext, node sqlparser.Expr, diff --git a/go/vt/vtgate/planbuilder/subquerytree.go b/go/vt/vtgate/planbuilder/subquerytree.go index 8b9ae39628c..026dbf0ddd6 100644 --- a/go/vt/vtgate/planbuilder/subquerytree.go +++ b/go/vt/vtgate/planbuilder/subquerytree.go @@ -20,17 +20,13 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/semantics" ) type subqueryTree struct { - subquery *sqlparser.Select outer queryTree inner queryTree - opcode engine.PulloutOpcode - argName string - hasValues string + extracted *sqlparser.ExtractedSubquery } var _ queryTree = (*subqueryTree)(nil) @@ -45,12 +41,9 @@ func (s *subqueryTree) cost() int { func (s *subqueryTree) clone() queryTree { result := &subqueryTree{ - subquery: s.subquery, outer: s.outer.clone(), inner: s.inner.clone(), - opcode: s.opcode, - argName: s.argName, - hasValues: s.hasValues, + extracted: s.extracted, } return result } diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index d7176c4e70d..592b0e38fa3 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1308,7 +1308,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select u.m from `user` as u where 1 != 1", - "Query": "select u.m from `user` as u where u.id in ::__vals and u.id in (select m2 from `user` where `user`.id = u.id and `user`.col = :user_extra_col)", + "Query": "select u.m from `user` as u where u.id in (select m2 from `user` where `user`.id = u.id and `user`.col = :user_extra_col) and u.id in ::__vals", "Table": "`user`", "Values": [ [ diff --git a/go/vt/vtgate/semantics/binder.go b/go/vt/vtgate/semantics/binder.go index cdd94cb139b..4eb7d127925 100644 --- a/go/vt/vtgate/semantics/binder.go +++ b/go/vt/vtgate/semantics/binder.go @@ -35,8 +35,8 @@ type binder struct { tc *tableCollector org originable typer *typer - subqueryMap map[*sqlparser.Select][]*subquery - subqueryRef map[*sqlparser.Subquery]*subquery + subqueryMap map[*sqlparser.Select][]*sqlparser.ExtractedSubquery + subqueryRef map[*sqlparser.Subquery]*sqlparser.ExtractedSubquery } func newBinder(scoper *scoper, org originable, tc *tableCollector, typer *typer) *binder { @@ -47,8 +47,8 @@ func newBinder(scoper *scoper, org originable, tc *tableCollector, typer *typer) org: org, tc: tc, typer: typer, - subqueryMap: map[*sqlparser.Select][]*subquery{}, - subqueryRef: map[*sqlparser.Subquery]*subquery{}, + subqueryMap: map[*sqlparser.Select][]*sqlparser.ExtractedSubquery{}, + subqueryRef: map[*sqlparser.Subquery]*sqlparser.ExtractedSubquery{}, } } @@ -60,6 +60,7 @@ func (b *binder) down(cursor *sqlparser.Cursor) error { return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] unable to bind subquery to select statement") } opcode := engine.PulloutValue + var original sqlparser.Expr = node switch par := cursor.Parent().(type) { case *sqlparser.ComparisonExpr: switch par.Operator { @@ -68,12 +69,15 @@ func (b *binder) down(cursor *sqlparser.Cursor) error { case sqlparser.NotInOp: opcode = engine.PulloutNotIn } + original = par case *sqlparser.ExistsExpr: opcode = engine.PulloutExists + original = par } - sq := &subquery{ - SubQuery: node, - OpCode: opcode, + sq := &sqlparser.ExtractedSubquery{ + Subquery: node.Select, + OpCode: int(opcode), + Original: sqlparser.CloneExpr(original), } b.subqueryMap[currScope.selectStmt] = append(b.subqueryMap[currScope.selectStmt], sq) b.subqueryRef[node] = sq diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go index 84720fa7a15..55dbfb55358 100644 --- a/go/vt/vtgate/semantics/scoper.go +++ b/go/vt/vtgate/semantics/scoper.go @@ -159,7 +159,7 @@ func (s *scoper) up(cursor *sqlparser.Cursor) error { return nil } -func validAsMapKey(s sqlparser.SQLNode) bool { +func ValidAsMapKey(s sqlparser.SQLNode) bool { return reflect.TypeOf(s).Comparable() } diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index b664c52b8ea..ce99cfc280b 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -22,7 +22,6 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/vindexes" "vitess.io/vitess/go/vt/sqlparser" @@ -85,8 +84,8 @@ type ( exprTypes map[sqlparser.Expr]querypb.Type selectScope map[*sqlparser.Select]*scope Comments sqlparser.Comments - SubqueryMap map[*sqlparser.Select][]*subquery - SubqueryRef map[*sqlparser.Subquery]*subquery + SubqueryMap map[*sqlparser.Select][]*sqlparser.ExtractedSubquery + SubqueryRef map[*sqlparser.Subquery]*sqlparser.ExtractedSubquery // ColumnEqualities is used to enable transitive closures // if a == b and b == c then a == c @@ -98,18 +97,6 @@ type ( ColumnName string } - subquery struct { - ArgName string - HasValues string - SubQuery *sqlparser.Subquery - OpCode engine.PulloutOpcode - - // ExprsNeedReplace list all the expressions that, if the subquery is later rewritten, need to - // be removed and replaced by ReplaceBy. - ExprsNeedReplace []sqlparser.Expr - ReplaceBy sqlparser.Expr - } - // SchemaInformation is used tp provide table information from Vschema. SchemaInformation interface { FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) @@ -230,7 +217,7 @@ func (d ExprDependencies) Dependencies(expr sqlparser.Expr) TableSet { // that have already set dependencies. _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { expr, ok := node.(sqlparser.Expr) - if !ok || !validAsMapKey(expr) { + if !ok || !ValidAsMapKey(expr) { // if this is not an expression, or it is an expression we can't use as a map-key, // just carry on down the tree return true, nil diff --git a/go/vt/vtgate/semantics/tabletset.go b/go/vt/vtgate/semantics/tabletset.go index 54aab332ebc..813a953fd11 100644 --- a/go/vt/vtgate/semantics/tabletset.go +++ b/go/vt/vtgate/semantics/tabletset.go @@ -97,6 +97,13 @@ func (ts *largeTableSet) mergeSmall(small uint64) *largeTableSet { return &largeTableSet{merged} } +func (ts *largeTableSet) removeSmall(small uint64) *largeTableSet { + merged := make([]uint64, len(ts.tables)) + copy(merged, ts.tables) + merged[0] &= ^small + return &largeTableSet{merged} +} + func (ts *largeTableSet) mergeInPlace(other *largeTableSet) { if len(other.tables) > len(ts.tables) { merged := make([]uint64, len(other.tables)) @@ -108,10 +115,20 @@ func (ts *largeTableSet) mergeInPlace(other *largeTableSet) { } } +func (ts *largeTableSet) removeInPlace(other *largeTableSet) { + for i := range ts.tables { + ts.tables[i] &= ^other.tables[i] + } +} + func (ts *largeTableSet) mergeSmallInPlace(small uint64) { ts.tables[0] |= small } +func (ts *largeTableSet) removeSmallInPlace(small uint64) { + ts.tables[0] &= ^small +} + func (ts *largeTableSet) tableOffset() (offset int) { var found bool for chunk, t := range ts.tables { @@ -283,6 +300,20 @@ func (ts *TableSet) MergeInPlace(other TableSet) { } } +// RemoveInPlace removes all the tables in `other` from this TableSet +func (ts *TableSet) RemoveInPlace(other TableSet) { + switch { + case ts.large == nil && other.large == nil: + ts.small &= ^other.small + case ts.large == nil: + ts.large = other.large.removeSmall(ts.small) + case other.large == nil: + ts.large.removeSmallInPlace(other.small) + default: + ts.large.removeInPlace(other.large) + } +} + // AddTable adds the given table to this set func (ts *TableSet) AddTable(tableidx int) { switch { diff --git a/go/vt/vtgate/semantics/tabletset_test.go b/go/vt/vtgate/semantics/tabletset_test.go index c901f23974e..0562c3278a2 100644 --- a/go/vt/vtgate/semantics/tabletset_test.go +++ b/go/vt/vtgate/semantics/tabletset_test.go @@ -77,6 +77,14 @@ func TestTableSet_LargeTablesConstituents(t *testing.T) { assert.Equal(t, expected, ts.Constituents()) } +func TestRemoveSmall(t *testing.T) { + ts1 := SingleTableSet(1) + ts2 := SingleTableSet(2) + tsX := ts1.Merge(ts2) + tsX.RemoveInPlace(ts2) + assert.Equal(t, tsX.Constituents(), ts1.Constituents()) +} + func TestTabletSet_LargeMergeInPlace(t *testing.T) { const SetRange = 256 const Blocks = 64 From 0560febd487a202b49dc077271ede9fe5436af49 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 11 Oct 2021 10:47:08 +0200 Subject: [PATCH 02/19] allow planComparison to exit early when we know we will plan a SelectNone Co-authored-by: Florent Poinsard Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/gen4_planner.go | 6 +- go/vt/vtgate/planbuilder/rewrite.go | 10 ++- go/vt/vtgate/planbuilder/route_planning.go | 2 +- go/vt/vtgate/planbuilder/routetree.go | 75 +++++++------------ .../planbuilder/testdata/filter_cases.txt | 2 +- go/vt/vtgate/semantics/semantic_state.go | 10 +++ 6 files changed, 48 insertions(+), 57 deletions(-) diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index cd9e73893f4..6af9859898e 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -178,9 +178,9 @@ func newBuildSelectPlan(selStmt sqlparser.SelectStatement, reservedVars *sqlpars func newPlanningContext(reservedVars *sqlparser.ReservedVars, semTable *semantics.SemTable, vschema ContextVSchema) *planningContext { ctx := &planningContext{ - reservedVars: reservedVars, - semTable: semTable, - vschema: vschema, + reservedVars: reservedVars, + semTable: semTable, + vschema: vschema, } return ctx } diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index be8081096c5..1d6cf13314f 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -89,6 +89,13 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { // replace the table name with the original table tableName.Name = vindexTable.Name node.Expr = tableName + case *sqlparser.Subquery: + r.isInSubquery++ + err := rewriteSubquery(cursor, r, node) + if err != nil { + r.err = err + } + } return true } @@ -144,10 +151,9 @@ func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subq if !found { return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } - if engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { + if semTableSQ.ArgName != "" || engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { return nil } - argName := r.reservedVars.ReserveSubQuery() semTableSQ.ArgName = argName cursor.Replace(semTableSQ) diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 1ed7fe4b659..0d2d2587f53 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -47,7 +47,7 @@ func (c planningContext) isSubQueryToReplace(e sqlparser.Expr) bool { if !ok { return false } - for _, subq := range c.reinsertSubQ{ + for _, subq := range c.reinsertSubQ { if subq == ext { return true } diff --git a/go/vt/vtgate/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index ec9e8792880..45321eecae0 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -192,18 +192,18 @@ func (rp *routeTree) searchForNewVindexes(ctx *planningContext, predicates []sql for _, filter := range predicates { switch node := filter.(type) { case *sqlparser.ExtractedSubquery: - comp, ok := node.Original.(*sqlparser.ComparisonExpr) + originalCmp, ok := node.Original.(*sqlparser.ComparisonExpr) if !ok { break } - found, err := rp.planComparison(ctx, comp) - if err != nil { + found, exitEarly, err := rp.planComparison(ctx, originalCmp) + if err != nil || exitEarly { return false, err } newVindexFound = newVindexFound || found case *sqlparser.ComparisonExpr: - found, err := rp.planComparison(ctx, node) - if err != nil { + found, exitEarly, err := rp.planComparison(ctx, node) + if err != nil || exitEarly { return false, err } newVindexFound = newVindexFound || found @@ -218,44 +218,44 @@ func (rp *routeTree) searchForNewVindexes(ctx *planningContext, predicates []sql return newVindexFound, nil } -func (rp *routeTree) planComparison(ctx *planningContext, node *sqlparser.ComparisonExpr) (bool, error) { +func (rp *routeTree) planComparison(ctx *planningContext, node *sqlparser.ComparisonExpr) (bool, bool, error) { if sqlparser.IsNull(node.Left) || sqlparser.IsNull(node.Right) { // we are looking at ANDed predicates in the WHERE clause. // since we know that nothing returns true when compared to NULL, // so we can safely bail out here rp.routeOpCode = engine.SelectNone - return false, nil + return false, true, nil } switch node.Operator { case sqlparser.EqualOp: found, err := rp.planEqualOp(ctx, node) if err != nil { - return false, err + return false, false, err } - return found, nil + return found, false, nil case sqlparser.InOp: if rp.isImpossibleIN(node) { - return false, nil + return false, false, nil } found, err := rp.planInOp(ctx, node) if err != nil { - return false, err + return false, false, err } - return found, nil + return found, false, nil case sqlparser.NotInOp: // NOT IN is always a scatter, except when we can be sure it would return nothing if rp.isImpossibleNotIN(node) { - return false, nil + return false, true, nil } case sqlparser.LikeOp: found, err := rp.planLikeOp(ctx, node) if err != nil { - return false, err + return false, false, err } - return found, nil + return found, false, nil } - return false, nil + return false, false, nil } func (rp *routeTree) isImpossibleIN(node *sqlparser.ComparisonExpr) bool { @@ -439,16 +439,15 @@ func (rp *routeTree) makePlanValue(ctx *planningContext, n sqlparser.Expr) (*sql for _, expr := range ctx.semTable.GetExprAndEqualities(n) { if subq, isSubq := expr.(*sqlparser.Subquery); isSubq { - for subquery, extractedSubquery := range ctx.semTable.SubqueryRef { - if sqlparser.EqualsRefOfSubquery(subquery, subq) { - switch engine.PulloutOpcode(extractedSubquery.OpCode) { - case engine.PulloutIn, engine.PulloutNotIn: - expr = sqlparser.NewListArg(extractedSubquery.ArgName) - case engine.PulloutValue, engine.PulloutExists: - expr = sqlparser.NewArgument(extractedSubquery.ArgName) - } - break - } + extractedSubquery := ctx.semTable.FindSubqueryReference(subq) + if extractedSubquery == nil { + continue + } + switch engine.PulloutOpcode(extractedSubquery.OpCode) { + case engine.PulloutIn, engine.PulloutNotIn: + expr = sqlparser.NewListArg(extractedSubquery.ArgName) + case engine.PulloutValue, engine.PulloutExists: + expr = sqlparser.NewArgument(extractedSubquery.ArgName) } } pv, err := makePlanValue(expr) @@ -483,19 +482,6 @@ func allNotNil(s []sqlparser.Expr) bool { return true } -// func (rp *routeTree) findMatchingColumnVindex(ctx *planningContext, column *sqlparser.ColName) bool { -// for _, pred := range rp.vindexPreds { -// if !ctx.semTable.DirectDeps(column).IsSolvedBy(pred.tableID) { -// continue -// } -// for _, vindexCol := range pred.colVindex.Columns { -// if column.Name.Equal(vindexCol) { -// -// } -// } -// } -// } - func (rp *routeTree) haveMatchingVindex( ctx *planningContext, node sqlparser.Expr, @@ -674,17 +660,6 @@ func makePlanValue(n sqlparser.Expr) (*sqltypes.PlanValue, error) { return &value, nil } -func argumentName(node sqlparser.SQLNode) string { - var argName string - switch node := node.(type) { - case sqlparser.ListArg: - argName = string(node) - case sqlparser.Argument: - argName = string(node) - } - return argName -} - // costFor returns a cost struct to make route choices easier to compare func costFor(foundVindex vindexes.Vindex, opcode engine.RouteOpcode) cost { switch opcode { diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index 592b0e38fa3..fe7e70a2b56 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -2143,7 +2143,7 @@ Gen4 plan same as above Gen4 plan same as above # Select with equals null -"select id from music where id = null" +"select id from music where id = 5 and id = null" { "QueryType": "SELECT", "Original": "select id from music where id = null", diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index ce99cfc280b..1252a6cda6c 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -260,3 +260,13 @@ func RewriteDerivedExpression(expr sqlparser.Expr, vt TableInfo) (sqlparser.Expr }, nil) return newExpr, nil } + +// FindSubqueryReference goes over the sub queries and searches for it by value equality instead of reference equality +func (st *SemTable) FindSubqueryReference(subquery *sqlparser.Subquery) *sqlparser.ExtractedSubquery { + for foundSubq, extractedSubquery := range st.SubqueryRef { + if sqlparser.EqualsRefOfSubquery(subquery, foundSubq) { + return extractedSubquery + } + } + return nil +} From 36141d21a92c8a4ed9a464f3c3f02adab5f42b42 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 11 Oct 2021 10:52:34 +0200 Subject: [PATCH 03/19] Update plan_test expectations Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/testdata/filter_cases.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index fe7e70a2b56..219d84bd877 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1399,7 +1399,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select u.m from `user` as u where 1 != 1", - "Query": "select u.m from `user` as u where u.id in ::__vals and u.id in (select m2 from `user` where `user`.id = u.id)", + "Query": "select u.m from `user` as u where u.id in (select m2 from `user` where `user`.id = u.id) and u.id in ::__vals", "Table": "`user`", "Values": [ [ @@ -1481,7 +1481,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select u.m from `user` as u where 1 != 1", - "Query": "select u.m from `user` as u where u.id = 5 and u.id in (select m2 from `user` where `user`.id = 5)", + "Query": "select u.m from `user` as u where u.id in (select m2 from `user` where `user`.id = 5) and u.id = 5", "Table": "`user`", "Values": [ 5 @@ -1569,7 +1569,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select u.m from `user` as u where 1 != 1", - "Query": "select u.m from `user` as u where u.id in ::__vals and u.id in (select m2 from `user` where `user`.id = u.id and `user`.col = :user_extra_col and `user`.id in (select m3 from user_extra where user_extra.user_id = `user`.id))", + "Query": "select u.m from `user` as u where u.id in (select m2 from `user` where `user`.id = u.id and `user`.col = :user_extra_col and `user`.id in (select m3 from user_extra where user_extra.user_id = `user`.id)) and u.id in ::__vals", "Table": "`user`", "Values": [ [ @@ -2143,7 +2143,7 @@ Gen4 plan same as above Gen4 plan same as above # Select with equals null -"select id from music where id = 5 and id = null" +"select id from music where id = null" { "QueryType": "SELECT", "Original": "select id from music where id = null", From efd95b22eb21acc109537c16fe16a1f111d02b4c Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 11 Oct 2021 11:09:42 +0200 Subject: [PATCH 04/19] Don't fail dependency logic on slices We should be able to handle calculating dependencies even if the expression is a slice and not valid as a map key. Co-authored-by: Florent Poinsard Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/abstract/concatenate.go | 2 +- go/vt/vtgate/planbuilder/abstract/operator.go | 9 --------- go/vt/vtgate/planbuilder/abstract/querygraph.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 16 +++++++++++----- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/concatenate.go b/go/vt/vtgate/planbuilder/abstract/concatenate.go index 48e40bff162..0cd3b231c72 100644 --- a/go/vt/vtgate/planbuilder/abstract/concatenate.go +++ b/go/vt/vtgate/planbuilder/abstract/concatenate.go @@ -23,7 +23,7 @@ import ( "vitess.io/vitess/go/vt/vtgate/semantics" ) -// Concatenate represents a UNION ALL. +// Concatenate represents a UNION ALL/DISTINCT. type Concatenate struct { Distinct bool SelectStmts []*sqlparser.Select diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index bb01f7bf92b..93e1c88a036 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -25,15 +25,6 @@ import ( type ( // Operator forms the tree of operators, representing the declarative query provided. - // An operator can be: - // * Derived - which represents an expression that generates a table. - // * QueryGraph - which represents a group of tables and predicates that can be evaluated in any order - // while still preserving the results - // * LeftJoin - A left join. These can't be evaluated in any order, so we keep them separate - // * Join - A join represents inner join. - // * SubQuery - Represents a query that encapsulates one or more sub-queries (SubQueryInner). - // * Vindex - Represents a query that selects from vindex tables. - // * Concatenate - Represents concatenation of the outputs of all the input sources Operator interface { // TableID returns a TableSet of the tables contained within TableID() semantics.TableSet diff --git a/go/vt/vtgate/planbuilder/abstract/querygraph.go b/go/vt/vtgate/planbuilder/abstract/querygraph.go index 6e03407e32d..5b63e2e201d 100644 --- a/go/vt/vtgate/planbuilder/abstract/querygraph.go +++ b/go/vt/vtgate/planbuilder/abstract/querygraph.go @@ -138,7 +138,7 @@ func (qg *QueryGraph) collectPredicate(predicate sqlparser.Expr, semTable *seman // we don't want to look at the subquery dependencies if extracted.OtherSide == nil { deps = semantics.SingleTableSet(0) - } else if semantics.ValidAsMapKey(extracted.OtherSide) { + } else { deps = semTable.RecursiveDeps(extracted.OtherSide) } } diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 1252a6cda6c..e4aff547405 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -206,10 +206,17 @@ func (st *SemTable) TypeFor(e sqlparser.Expr) *querypb.Type { } // Dependencies return the table dependencies of the expression. This method finds table dependencies recursively -func (d ExprDependencies) Dependencies(expr sqlparser.Expr) TableSet { - deps, found := d[expr] - if found { - return deps +func (d ExprDependencies) Dependencies(expr sqlparser.Expr) (deps TableSet) { + if ValidAsMapKey(expr) { + // we have something that could live in the cache + var found bool + deps, found = d[expr] + if found { + return deps + } + defer func() { + d[expr] = deps + }() } // During the original semantic analysis, all ColName:s were found and bound the the corresponding tables @@ -232,7 +239,6 @@ func (d ExprDependencies) Dependencies(expr sqlparser.Expr) TableSet { return !found, nil }, expr) - d[expr] = deps return deps } From 5497971f16a0b0768c359ee05df0b45fdbb657f2 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 12:57:31 +0200 Subject: [PATCH 05/19] support for ::__vals argument when using IN subqueries Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/querytree_transformers.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index a30922627cf..ad2f68ac92f 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -314,6 +314,12 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { if predicate.Operator == sqlparser.InOp { switch predicate.Left.(type) { case *sqlparser.ColName: + if subq, isSubq := predicate.Right.(*sqlparser.Subquery); isSubq { + extractedSubquery := ctx.semTable.FindSubqueryReference(subq) + if extractedSubquery != nil { + extractedSubquery.ArgName = engine.ListVarName + } + } predicate.Right = sqlparser.ListArg(engine.ListVarName) } } From 0573e46e7f0c2ef3ca9563dde0b22cdaf64590b3 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 13:02:57 +0200 Subject: [PATCH 06/19] fix impossible IN operation Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/routetree.go | 2 +- go/vt/vtgate/planbuilder/testdata/onecase.txt | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index 45321eecae0..9a6c5aea6aa 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -236,7 +236,7 @@ func (rp *routeTree) planComparison(ctx *planningContext, node *sqlparser.Compar return found, false, nil case sqlparser.InOp: if rp.isImpossibleIN(node) { - return false, false, nil + return false, true, nil } found, err := rp.planInOp(ctx, node) if err != nil { diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.txt b/go/vt/vtgate/planbuilder/testdata/onecase.txt index e819513f354..08bb8154ecf 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.txt +++ b/go/vt/vtgate/planbuilder/testdata/onecase.txt @@ -1 +1,19 @@ # Add your test case here for debugging and run go test -run=One. +# Single table with unique vindex match and IN (null) +"select id from music where user_id = 4 and id IN (null)" +{ + "QueryType": "SELECT", + "Original": "select id from music where user_id = 4 and id IN (null)", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectNone", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from music where 1 != 1", + "Query": "select id from music where user_id = 4 and id in (null)", + "Table": "music" + } +} +Gen4 plan same as above From 148513ee77f7a7e93daf38b5a954b6a8f09b4bd0 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 13:24:45 +0200 Subject: [PATCH 07/19] using rewritten subquery to search new vindexes Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/routetree.go | 9 ++++++++- go/vt/vtgate/planbuilder/testdata/onecase.txt | 18 ------------------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/go/vt/vtgate/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index 9a6c5aea6aa..c738d410aae 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -196,7 +196,14 @@ func (rp *routeTree) searchForNewVindexes(ctx *planningContext, predicates []sql if !ok { break } - found, exitEarly, err := rp.planComparison(ctx, originalCmp) + + // using the node.subquery which is the rewritten version of our subquery + cmp := &sqlparser.ComparisonExpr{ + Left: node.OtherSide, + Right: &sqlparser.Subquery{Select: node.Subquery}, + Operator: originalCmp.Operator, + } + found, exitEarly, err := rp.planComparison(ctx, cmp) if err != nil || exitEarly { return false, err } diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.txt b/go/vt/vtgate/planbuilder/testdata/onecase.txt index 08bb8154ecf..e819513f354 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.txt +++ b/go/vt/vtgate/planbuilder/testdata/onecase.txt @@ -1,19 +1 @@ # Add your test case here for debugging and run go test -run=One. -# Single table with unique vindex match and IN (null) -"select id from music where user_id = 4 and id IN (null)" -{ - "QueryType": "SELECT", - "Original": "select id from music where user_id = 4 and id IN (null)", - "Instructions": { - "OperatorType": "Route", - "Variant": "SelectNone", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select id from music where 1 != 1", - "Query": "select id from music where user_id = 4 and id in (null)", - "Table": "music" - } -} -Gen4 plan same as above From 41541b0b7536fc5613d5ae6cd78a6b39cc67f91c Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 15:18:11 +0200 Subject: [PATCH 08/19] fix issue with IN subqueries becoming scatter when merging them Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/gen4_planner.go | 2 +- .../planbuilder/querytree_transformers.go | 23 +++++++++++-------- go/vt/vtgate/planbuilder/route_planning.go | 8 +++---- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 6af9859898e..d990bf3bba2 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -215,7 +215,7 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat sel: node, } - replaceSubQuery(ctx.reinsertSubQ, node) + replaceSubQuery(ctx, node) var err error plan, err = hp.planHorizon(ctx, plan) if err != nil { diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index ad2f68ac92f..ce22cdb92fa 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -387,7 +387,7 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { Comments: ctx.semTable.Comments, } - replaceSubQuery(ctx.reinsertSubQ, sel) + replaceSubQuery(ctx, sel) // TODO clean up when gen4 is the only planner var condition sqlparser.Expr @@ -522,14 +522,19 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { return true } -func replaceSubQuery(exprToReplaceBySqExpr []*sqlparser.ExtractedSubquery, sel *sqlparser.Select) { - if len(exprToReplaceBySqExpr) > 0 { - sqr := &subQReplacer{exprToReplaceBySqExpr: exprToReplaceBySqExpr} +func replaceSubQuery(ctx *planningContext, sel *sqlparser.Select) { + if len(ctx.reinsertSubQ) == 0 { + return + } + extractedSubqueries := []*sqlparser.ExtractedSubquery{} + for _, subquery := range ctx.reinsertSubQ { + extractedSubqueries = append(extractedSubqueries, ctx.semTable.FindSubqueryReference(subquery)) + } + sqr := &subQReplacer{exprToReplaceBySqExpr: extractedSubqueries} + sqlparser.Rewrite(sel, sqr.replacer, nil) + for sqr.replaced { + // to handle subqueries inside subqueries, we need to do this again and again until no replacements are left + sqr.replaced = false sqlparser.Rewrite(sel, sqr.replacer, nil) - for sqr.replaced { - // to handle subqueries inside subqueries, we need to do this again and again until no replacements are left - sqr.replaced = false - sqlparser.Rewrite(sel, sqr.replacer, nil) - } } } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 0d2d2587f53..b4c48ede935 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -39,16 +39,16 @@ type planningContext struct { semTable *semantics.SemTable vschema ContextVSchema // these help in replacing the argNames with the subquery - reinsertSubQ []*sqlparser.ExtractedSubquery + reinsertSubQ []*sqlparser.Subquery } func (c planningContext) isSubQueryToReplace(e sqlparser.Expr) bool { - ext, ok := e.(*sqlparser.ExtractedSubquery) + ext, ok := e.(*sqlparser.Subquery) if !ok { return false } for _, subq := range c.reinsertSubQ { - if subq == ext { + if sqlparser.EqualsRefOfSubquery(subq, ext) { return true } } @@ -274,7 +274,7 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre } func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { - ctx.reinsertSubQ = append(ctx.reinsertSubQ, subq.ExtractedSubquery) + ctx.reinsertSubQ = append(ctx.reinsertSubQ, &sqlparser.Subquery{Select: subq.ExtractedSubquery.Subquery}) // go over the subquery and add its tables to the one's solved by the route it is merged with // this is needed to so that later when we try to push projections, we get the correct From 2483b11967edb1b5f037b2e11691582614573502 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 15:35:48 +0200 Subject: [PATCH 09/19] changed the way we calculate dependencies for ExtractedSubqueries Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/abstract/querygraph.go | 9 --------- go/vt/vtgate/semantics/semantic_state.go | 11 ++++++++--- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/querygraph.go b/go/vt/vtgate/planbuilder/abstract/querygraph.go index 5b63e2e201d..7f5db077309 100644 --- a/go/vt/vtgate/planbuilder/abstract/querygraph.go +++ b/go/vt/vtgate/planbuilder/abstract/querygraph.go @@ -133,15 +133,6 @@ func (qg *QueryGraph) addJoinPredicates(ts semantics.TableSet, expr sqlparser.Ex func (qg *QueryGraph) collectPredicate(predicate sqlparser.Expr, semTable *semantics.SemTable) error { deps := semTable.RecursiveDeps(predicate) - extracted, ok := predicate.(*sqlparser.ExtractedSubquery) - if ok { - // we don't want to look at the subquery dependencies - if extracted.OtherSide == nil { - deps = semantics.SingleTableSet(0) - } else { - deps = semTable.RecursiveDeps(extracted.OtherSide) - } - } switch deps.NumberOfTables() { case 0: qg.addNoDepsPredicate(predicate) diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index e4aff547405..28474e47609 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -230,10 +230,15 @@ func (d ExprDependencies) Dependencies(expr sqlparser.Expr) (deps TableSet) { return true, nil } - set, found := d[expr] - if found { - deps.MergeInPlace(set) + if extracted, ok := expr.(*sqlparser.ExtractedSubquery); ok { + if extracted.OtherSide != nil { + set := d.Dependencies(extracted.OtherSide) + deps.MergeInPlace(set) + } + return false, nil } + set, found := d[expr] + deps.MergeInPlace(set) // if we found a cached value, there is no need to continue down to visit children return !found, nil From f31d4c3f232e303fbc300ad72deb0c92f8a9ddb2 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 11 Oct 2021 16:37:44 +0200 Subject: [PATCH 10/19] add ExtractedSubquery to the precedence logic Signed-off-by: Andres Taylor --- go/vt/sqlparser/precedence.go | 2 ++ go/vt/sqlparser/precedence_test.go | 23 +++++++++++++++++++++++ go/vt/sqlparser/tracked_buffer.go | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/go/vt/sqlparser/precedence.go b/go/vt/sqlparser/precedence.go index 7b5fb7ace4f..b6e7397a6a0 100644 --- a/go/vt/sqlparser/precedence.go +++ b/go/vt/sqlparser/precedence.go @@ -90,6 +90,8 @@ func precedenceFor(in Expr) Precendence { } case *IntervalExpr: return P1 + case *ExtractedSubquery: + return precedenceFor(node.Original) } return Syntactic diff --git a/go/vt/sqlparser/precedence_test.go b/go/vt/sqlparser/precedence_test.go index 20ef03e0d3e..7ad31f822fe 100644 --- a/go/vt/sqlparser/precedence_test.go +++ b/go/vt/sqlparser/precedence_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -65,6 +66,28 @@ func TestAndOrPrecedence(t *testing.T) { } } +func TestNotInSubqueryPrecedence(t *testing.T) { + tree, err := Parse("select * from a where not id in (select 42)") + require.NoError(t, err) + not := tree.(*Select).Where.Expr.(*NotExpr) + cmp := not.Expr.(*ComparisonExpr) + subq := cmp.Right.(*Subquery) + fmt.Println(subq) + + extracted := &ExtractedSubquery{ + Original: cmp, + ArgName: "arg1", + HasValuesArg: "has_values1", + OpCode: 1, + Subquery: subq.Select, + OtherSide: cmp.Left, + } + not.Expr = extracted + + output := readable(not) + assert.Equal(t, "not (:has_values1 = 1 and id in ::arg1)", output) +} + func TestPlusStarPrecedence(t *testing.T) { validSQL := []struct { input string diff --git a/go/vt/sqlparser/tracked_buffer.go b/go/vt/sqlparser/tracked_buffer.go index 30ef64a5bdd..c1983dd66b4 100644 --- a/go/vt/sqlparser/tracked_buffer.go +++ b/go/vt/sqlparser/tracked_buffer.go @@ -169,7 +169,7 @@ func (buf *TrackedBuffer) formatter(node SQLNode) { } } -//needParens says if we need a parenthesis +// needParens says if we need a parenthesis // op is the operator we are printing // val is the value we are checking if we need parens around or not // left let's us know if the value is on the lhs or rhs of the operator From e12a286b06292220c619cf87bf1842a855109b85 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 19:12:46 +0200 Subject: [PATCH 11/19] Updated plan tests with the new ExtractedSubquery precedence logic Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/testdata/filter_cases.txt | 12 ++++++------ go/vt/vtgate/planbuilder/testdata/from_cases.txt | 8 ++++---- .../planbuilder/testdata/postprocess_cases.txt | 10 +++++----- go/vt/vtgate/planbuilder/testdata/tpch_cases.txt | 2 +- go/vt/vtgate/planbuilder/testdata/wireup_cases.txt | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index 219d84bd877..61339d7a816 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1760,7 +1760,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", + "Query": "select id from `user` where (:__sq_has_values1 = 1 and id in ::__vals)", "Table": "`user`", "Values": [ "::__sq1" @@ -1999,7 +1999,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id2 from `user` where 1 != 1", - "Query": "select id2 from `user` where :__sq_has_values2 = 1 and id2 in ::__sq2", + "Query": "select id2 from `user` where (:__sq_has_values2 = 1 and id2 in ::__sq2)", "Table": "`user`" } ] @@ -2414,7 +2414,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where not (:__sq_has_values1 = 1 and id in ::__sq1) and :__sq_has_values2 = 1 and id in ::__vals", + "Query": "select id from `user` where not (:__sq_has_values1 = 1 and id in ::__sq1) and (:__sq_has_values2 = 1 and id in ::__vals)", "Table": "`user`", "Values": [ "::__sq2" @@ -2574,7 +2574,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", + "Query": "select id from `user` where (:__sq_has_values1 = 1 and id in ::__vals)", "Table": "`user`", "Values": [ "::__sq1" @@ -2828,7 +2828,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 5) and :__sq_has_values2 = 1 and id in ::__sq2", + "Query": "select id from `user` where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 5) and (:__sq_has_values2 = 1 and id in ::__sq2)", "Table": "`user`", "Values": [ 5 @@ -3260,7 +3260,7 @@ Gen4 plan same as above }, "FieldQuery": "select `user`.id, `user`.col, weight_string(`user`.id), weight_string(`user`.col) from `user` where 1 != 1", "OrderBy": "(0|2) ASC, (1|3) ASC", - "Query": "select `user`.id, `user`.col, weight_string(`user`.id), weight_string(`user`.col) from `user` where :__sq_has_values1 = 1 and `user`.col in ::__sq1 order by `user`.id asc, `user`.col asc", + "Query": "select `user`.id, `user`.col, weight_string(`user`.id), weight_string(`user`.col) from `user` where (:__sq_has_values1 = 1 and `user`.col in ::__sq1) order by `user`.id asc, `user`.col asc", "Table": "`user`" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index 3f540ef02f9..b477e8d0933 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt @@ -2955,7 +2955,7 @@ Gen4 plan same as above "Sharded": false }, "FieldQuery": "select unsharded_a.col from unsharded_a, unsharded_b where 1 != 1", - "Query": "select unsharded_a.col from unsharded_a, unsharded_b where :__sq_has_values1 = 1 and unsharded_a.col in ::__sq1", + "Query": "select unsharded_a.col from unsharded_a, unsharded_b where (:__sq_has_values1 = 1 and unsharded_a.col in ::__sq1)", "Table": "unsharded_a, unsharded_b" } ] @@ -3058,7 +3058,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select 1 from `user` where 1 != 1", - "Query": "select 1 from `user` where :__sq_has_values1 = 1 and `user`.col in ::__sq1", + "Query": "select 1 from `user` where (:__sq_has_values1 = 1 and `user`.col in ::__sq1)", "Table": "`user`" } ] @@ -3164,7 +3164,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select 1 from `user` where 1 != 1", - "Query": "select 1 from `user` where :__sq_has_values1 = 1 and `user`.col in ::__sq1", + "Query": "select 1 from `user` where (:__sq_has_values1 = 1 and `user`.col in ::__sq1)", "Table": "`user`" } ] @@ -3278,7 +3278,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select 1 from `user` where 1 != 1", - "Query": "select 1 from `user` where :__sq_has_values1 = 1 and `user`.col in ::__sq1", + "Query": "select 1 from `user` where (:__sq_has_values1 = 1 and `user`.col in ::__sq1)", "Table": "`user`" }, { diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index 150d3bd3b19..a674a78de97 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt @@ -243,7 +243,7 @@ Gen4 error: Column 'col1' in field list is ambiguous "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", + "Query": "select id from `user` where (:__sq_has_values1 = 1 and id in ::__vals)", "Table": "`user`", "Values": [ "::__sq1" @@ -604,7 +604,7 @@ Gen4 plan same as above }, "FieldQuery": "select col, weight_string(col) from `user` where 1 != 1", "OrderBy": "(0|1) ASC", - "Query": "select col, weight_string(col) from `user` where :__sq_has_values1 = 1 and col in ::__sq1 order by col asc", + "Query": "select col, weight_string(col) from `user` where (:__sq_has_values1 = 1 and col in ::__sq1) order by col asc", "ResultColumns": 1, "Table": "`user`" } @@ -953,7 +953,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select col from `user` where 1 != 1", - "Query": "select col from `user` where :__sq_has_values1 = 1 and col in ::__sq1 order by null", + "Query": "select col from `user` where (:__sq_has_values1 = 1 and col in ::__sq1) order by null", "Table": "`user`" } ] @@ -1133,7 +1133,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select col from `user` where 1 != 1", - "Query": "select col from `user` where :__sq_has_values1 = 1 and col in ::__sq1 order by rand()", + "Query": "select col from `user` where (:__sq_has_values1 = 1 and col in ::__sq1) order by rand()", "Table": "`user`" } ] @@ -1689,7 +1689,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select col from `user` where 1 != 1", - "Query": "select col from `user` where :__sq_has_values1 = 1 and col in ::__sq1 limit :__upper_limit", + "Query": "select col from `user` where (:__sq_has_values1 = 1 and col in ::__sq1) limit :__upper_limit", "Table": "`user`" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt b/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt index 25a7e29c634..2cef6bd4d3c 100644 --- a/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt @@ -807,7 +807,7 @@ Gen4 error: unsupported: cross-shard correlated subquery }, "FieldQuery": "select o_custkey, o_orderkey, o_orderdate, o_totalprice, weight_string(o_orderkey), weight_string(o_orderdate), weight_string(o_totalprice) from orders where 1 != 1", "OrderBy": "(3|6) DESC, (2|5) ASC", - "Query": "select o_custkey, o_orderkey, o_orderdate, o_totalprice, weight_string(o_orderkey), weight_string(o_orderdate), weight_string(o_totalprice) from orders where :__sq_has_values1 = 1 and o_orderkey in ::__vals order by o_totalprice desc, o_orderdate asc", + "Query": "select o_custkey, o_orderkey, o_orderdate, o_totalprice, weight_string(o_orderkey), weight_string(o_orderdate), weight_string(o_totalprice) from orders where (:__sq_has_values1 = 1 and o_orderkey in ::__vals) order by o_totalprice desc, o_orderdate asc", "Table": "orders", "Values": [ "::__sq1" diff --git a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt index 708c1cb4ade..b9cc301eb71 100644 --- a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt @@ -1185,7 +1185,7 @@ "Sharded": true }, "FieldQuery": "select 1 from `user` where 1 != 1", - "Query": "select 1 from `user` where :__sq_has_values1 = 1 and id in ::__vals", + "Query": "select 1 from `user` where (:__sq_has_values1 = 1 and id in ::__vals)", "Table": "`user`", "Values": [ "::__sq1" From dc15f6104905462ea78be8cbb3915f16de886b44 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 11 Oct 2021 19:55:15 +0200 Subject: [PATCH 12/19] Fixed ExtractedSuqbuery issue with routed tables Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/rewrite.go | 30 +++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index 1d6cf13314f..de7ca445949 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -25,10 +25,10 @@ import ( ) type rewriter struct { - semTable *semantics.SemTable - reservedVars *sqlparser.ReservedVars - isInSubquery int - err error + semTable *semantics.SemTable + reservedVars *sqlparser.ReservedVars + subqueryStack []*sqlparser.ExtractedSubquery + err error } func queryRewrite(semTable *semantics.SemTable, reservedVars *sqlparser.ReservedVars, statement sqlparser.SelectStatement) error { @@ -58,7 +58,7 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { case *sqlparser.AliasedTableExpr: // rewrite names of the routed tables for the subquery // We only need to do this for non-derived tables and if they are in a subquery - if _, isDerived := node.Expr.(*sqlparser.DerivedTable); isDerived || r.isInSubquery == 0 { + if _, isDerived := node.Expr.(*sqlparser.DerivedTable); isDerived || len(r.subqueryStack) == 0 { break } // find the tableSet and tableInfo that this table points to @@ -89,13 +89,24 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { // replace the table name with the original table tableName.Name = vindexTable.Name node.Expr = tableName + + if len(r.subqueryStack) == 0 { + break + } + currSuqbuery := r.subqueryStack[len(r.subqueryStack)-1] + sqlparser.Rewrite(currSuqbuery.Original, func(cursor *sqlparser.Cursor) bool { + switch cursor.Node().(type) { + case *sqlparser.Subquery: + cursor.Replace(&sqlparser.Subquery{Select: currSuqbuery.Subquery}) + return false + } + return true + }, nil) case *sqlparser.Subquery: - r.isInSubquery++ err := rewriteSubquery(cursor, r, node) if err != nil { r.err = err } - } return true } @@ -103,7 +114,7 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { func (r *rewriter) rewriteUp(cursor *sqlparser.Cursor) bool { switch cursor.Node().(type) { case *sqlparser.Subquery: - r.isInSubquery-- + r.subqueryStack = r.subqueryStack[:len(r.subqueryStack)-1] } return r.err == nil } @@ -125,6 +136,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } + r.subqueryStack = append(r.subqueryStack, semTableSQ) argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName semTableSQ.HasValuesArg = hasValuesArg @@ -154,6 +166,7 @@ func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subq if semTableSQ.ArgName != "" || engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { return nil } + r.subqueryStack = append(r.subqueryStack, semTableSQ) argName := r.reservedVars.ReserveSubQuery() semTableSQ.ArgName = argName cursor.Replace(semTableSQ) @@ -166,6 +179,7 @@ func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlpars return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } + r.subqueryStack = append(r.subqueryStack, semTableSQ) argName := r.reservedVars.ReserveHasValuesSubQuery() semTableSQ.ArgName = argName cursor.Replace(semTableSQ) From 1f2bb8d451405b85d225728a337a2ba15962a846 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 09:39:59 +0200 Subject: [PATCH 13/19] fixed issue when dealing with two subqueries that are equal Signed-off-by: Florent Poinsard --- go/vt/sqlparser/ast.go | 1 + go/vt/sqlparser/ast_equals.go | 1 + go/vt/vtgate/planbuilder/querytree_transformers.go | 7 ++----- go/vt/vtgate/planbuilder/route_planning.go | 9 +++------ go/vt/vtgate/semantics/semantic_state.go | 10 ++++++++++ 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 38e713e8cb1..820ba36707f 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -1973,6 +1973,7 @@ type ( OpCode int // this should really be engine.PulloutOpCode, but we cannot depend on engine :( Subquery SelectStatement OtherSide Expr // represents the side of the comparison, this field will be nil if Original is not a comparison + NeedsRewrite bool } ) diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go index 04a1e6cc654..155409ac6b1 100644 --- a/go/vt/sqlparser/ast_equals.go +++ b/go/vt/sqlparser/ast_equals.go @@ -1566,6 +1566,7 @@ func EqualsRefOfExtractedSubquery(a, b *ExtractedSubquery) bool { return a.ArgName == b.ArgName && a.HasValuesArg == b.HasValuesArg && a.OpCode == b.OpCode && + a.NeedsRewrite == b.NeedsRewrite && EqualsExpr(a.Original, b.Original) && EqualsSelectStatement(a.Subquery, b.Subquery) && EqualsExpr(a.OtherSide, b.OtherSide) diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index ce22cdb92fa..617da594bc8 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -523,13 +523,10 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { } func replaceSubQuery(ctx *planningContext, sel *sqlparser.Select) { - if len(ctx.reinsertSubQ) == 0 { + extractedSubqueries := ctx.semTable.GetSubqueryNeedingRewrite() + if len(extractedSubqueries) == 0 { return } - extractedSubqueries := []*sqlparser.ExtractedSubquery{} - for _, subquery := range ctx.reinsertSubQ { - extractedSubqueries = append(extractedSubqueries, ctx.semTable.FindSubqueryReference(subquery)) - } sqr := &subQReplacer{exprToReplaceBySqExpr: extractedSubqueries} sqlparser.Rewrite(sel, sqr.replacer, nil) for sqr.replaced { diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index b4c48ede935..0ddab3923df 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -38,8 +38,6 @@ type planningContext struct { reservedVars *sqlparser.ReservedVars semTable *semantics.SemTable vschema ContextVSchema - // these help in replacing the argNames with the subquery - reinsertSubQ []*sqlparser.Subquery } func (c planningContext) isSubQueryToReplace(e sqlparser.Expr) bool { @@ -47,12 +45,11 @@ func (c planningContext) isSubQueryToReplace(e sqlparser.Expr) bool { if !ok { return false } - for _, subq := range c.reinsertSubQ { - if sqlparser.EqualsRefOfSubquery(subq, ext) { + for _, extractedSubq := range c.semTable.GetSubqueryNeedingRewrite() { + if extractedSubq.NeedsRewrite && sqlparser.EqualsRefOfSubquery(&sqlparser.Subquery{Select: extractedSubq.Subquery}, ext) { return true } } - return false } @@ -274,7 +271,7 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre } func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { - ctx.reinsertSubQ = append(ctx.reinsertSubQ, &sqlparser.Subquery{Select: subq.ExtractedSubquery.Subquery}) + subq.ExtractedSubquery.NeedsRewrite = true // go over the subquery and add its tables to the one's solved by the route it is merged with // this is needed to so that later when we try to push projections, we get the correct diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 28474e47609..bd2a91e327e 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -281,3 +281,13 @@ func (st *SemTable) FindSubqueryReference(subquery *sqlparser.Subquery) *sqlpars } return nil } + +func (st *SemTable) GetSubqueryNeedingRewrite() []*sqlparser.ExtractedSubquery { + var res []*sqlparser.ExtractedSubquery + for _, extractedSubquery := range st.SubqueryRef { + if extractedSubquery.NeedsRewrite { + res = append(res, extractedSubquery) + } + } + return res +} From 96a780d4b3ee8e436055281646114b0f2ca35853 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 10:21:04 +0200 Subject: [PATCH 14/19] fixed issue with ExtractedSubquery's Original not mimicking the changes done on SelStmt Signed-off-by: Florent Poinsard --- go/vt/sqlparser/ast.go | 4 +-- go/vt/sqlparser/ast_format.go | 4 +-- go/vt/vtgate/planbuilder/rewrite.go | 29 +------------------ go/vt/vtgate/semantics/binder.go | 44 +++++++++++++++++++++-------- 4 files changed, 37 insertions(+), 44 deletions(-) diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index 820ba36707f..b724a8b375a 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -1967,13 +1967,13 @@ type ( // ExtractedSubquery is a subquery that has been extracted from the original AST // This is a struct that the parser will never produce - it's written and read by the gen4 planner ExtractedSubquery struct { - Original Expr + Original Expr // original expression that was replaced by this ExtractedSubquery ArgName string HasValuesArg string OpCode int // this should really be engine.PulloutOpCode, but we cannot depend on engine :( Subquery SelectStatement OtherSide Expr // represents the side of the comparison, this field will be nil if Original is not a comparison - NeedsRewrite bool + NeedsRewrite bool // tells whether we need to rewrite this subquery to Original or not } ) diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index fb85803211f..d86ca7f1b0e 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -1728,8 +1728,8 @@ func (node *ExtractedSubquery) Format(buf *TrackedBuffer) { case *ComparisonExpr: // other_side = :__sq cmp := &ComparisonExpr{ - Left: node.OtherSide, - Right: NewArgument(node.ArgName), + Left: node.OtherSide, + Right: NewArgument(node.ArgName), Operator: original.Operator, } var expr Expr = cmp diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index de7ca445949..830f38ed60f 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -89,19 +89,6 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { // replace the table name with the original table tableName.Name = vindexTable.Name node.Expr = tableName - - if len(r.subqueryStack) == 0 { - break - } - currSuqbuery := r.subqueryStack[len(r.subqueryStack)-1] - sqlparser.Rewrite(currSuqbuery.Original, func(cursor *sqlparser.Cursor) bool { - switch cursor.Node().(type) { - case *sqlparser.Subquery: - cursor.Replace(&sqlparser.Subquery{Select: currSuqbuery.Subquery}) - return false - } - return true - }, nil) case *sqlparser.Subquery: err := rewriteSubquery(cursor, r, node) if err != nil { @@ -126,7 +113,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co node.Operator != sqlparser.NotEqualOp { return nil } - subq, exp := getSubqueryAndOtherSide(node) + subq, exp := semantics.GetSubqueryAndOtherSide(node) if subq == nil || exp == nil { return nil } @@ -140,24 +127,10 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName semTableSQ.HasValuesArg = hasValuesArg - semTableSQ.OtherSide = exp cursor.Replace(semTableSQ) return nil } -func getSubqueryAndOtherSide(node *sqlparser.ComparisonExpr) (*sqlparser.Subquery, sqlparser.Expr) { - var subq *sqlparser.Subquery - var exp sqlparser.Expr - if lSubq, lIsSubq := node.Left.(*sqlparser.Subquery); lIsSubq { - subq = lSubq - exp = node.Right - } else if rSubq, rIsSubq := node.Right.(*sqlparser.Subquery); rIsSubq { - subq = rSubq - exp = node.Left - } - return subq, exp -} - func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) error { semTableSQ, found := r.semTable.SubqueryRef[node] if !found { diff --git a/go/vt/vtgate/semantics/binder.go b/go/vt/vtgate/semantics/binder.go index 4eb7d127925..fb29cf5554f 100644 --- a/go/vt/vtgate/semantics/binder.go +++ b/go/vt/vtgate/semantics/binder.go @@ -59,26 +59,33 @@ func (b *binder) down(cursor *sqlparser.Cursor) error { if currScope.selectStmt == nil { return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] unable to bind subquery to select statement") } - opcode := engine.PulloutValue - var original sqlparser.Expr = node + + sq := &sqlparser.ExtractedSubquery{ + Subquery: node.Select, + Original: node, + OpCode: int(engine.PulloutValue), + } + switch par := cursor.Parent().(type) { case *sqlparser.ComparisonExpr: switch par.Operator { case sqlparser.InOp: - opcode = engine.PulloutIn + sq.OpCode = int(engine.PulloutIn) case sqlparser.NotInOp: - opcode = engine.PulloutNotIn + sq.OpCode = int(engine.PulloutNotIn) + } + subq, exp := GetSubqueryAndOtherSide(par) + sq.Original = &sqlparser.ComparisonExpr{ + Left: exp, + Operator: par.Operator, + Right: subq, } - original = par + sq.OtherSide = exp case *sqlparser.ExistsExpr: - opcode = engine.PulloutExists - original = par - } - sq := &sqlparser.ExtractedSubquery{ - Subquery: node.Select, - OpCode: int(opcode), - Original: sqlparser.CloneExpr(original), + sq.OpCode = int(engine.PulloutExists) + sq.Original = par } + b.subqueryMap[currScope.selectStmt] = append(b.subqueryMap[currScope.selectStmt], sq) b.subqueryRef[node] = sq case *sqlparser.ColName: @@ -161,3 +168,16 @@ func makeAmbiguousError(colName *sqlparser.ColName, err error) error { } return err } + +func GetSubqueryAndOtherSide(node *sqlparser.ComparisonExpr) (*sqlparser.Subquery, sqlparser.Expr) { + var subq *sqlparser.Subquery + var exp sqlparser.Expr + if lSubq, lIsSubq := node.Left.(*sqlparser.Subquery); lIsSubq { + subq = lSubq + exp = node.Right + } else if rSubq, rIsSubq := node.Right.(*sqlparser.Subquery); rIsSubq { + subq = rSubq + exp = node.Left + } + return subq, exp +} From 90d924c9c34fdfc074b22d4eb4c086dedda52ef8 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 10:24:30 +0200 Subject: [PATCH 15/19] applied format and import Signed-off-by: Florent Poinsard --- go/vt/sqlparser/ast_format_fast.go | 6 +++--- go/vt/vtgate/planbuilder/abstract/operator_test.go | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go index a7febaceca5..f3c4aef9d20 100644 --- a/go/vt/sqlparser/ast_format_fast.go +++ b/go/vt/sqlparser/ast_format_fast.go @@ -2258,9 +2258,9 @@ func (node *ExtractedSubquery) formatFast(buf *TrackedBuffer) { case *ComparisonExpr: // other_side = :__sq cmp := &ComparisonExpr{ - Left: node.OtherSide, - Right: NewArgument(node.ArgName), - Operator: original.Operator, + Left: node.OtherSide, + Right: NewArgument(node.ArgName), + Operator: original.Operator, } var expr Expr = cmp switch original.Operator { diff --git a/go/vt/vtgate/planbuilder/abstract/operator_test.go b/go/vt/vtgate/planbuilder/abstract/operator_test.go index fc6edfe3d4a..704808b956c 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator_test.go +++ b/go/vt/vtgate/planbuilder/abstract/operator_test.go @@ -24,6 +24,7 @@ import ( "sort" "strings" "testing" + "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/vindexes" From ee3a800aa63b59208dd822be18fb1be24ee5b1f5 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 11:04:04 +0200 Subject: [PATCH 16/19] addition of unit test for semtable's subquery mapping Signed-off-by: Florent Poinsard --- go/vt/vtgate/semantics/analyzer_test.go | 86 +++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 3d43141c430..a0218a81f79 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -17,8 +17,11 @@ limitations under the License. package semantics import ( + "fmt" "testing" + "vitess.io/vitess/go/vt/vtgate/engine" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -510,6 +513,89 @@ func TestScopeForSubqueries(t *testing.T) { } } +func TestSubqueriesMappingWhereClause(t *testing.T) { + tcs := []struct { + sql string + opCode engine.PulloutOpcode + otherSideName string + }{ + { + sql: "select id from t1 where id in (select uid from t2)", + opCode: engine.PulloutIn, + otherSideName: "id", + }, + { + sql: "select id from t1 where id not in (select uid from t2)", + opCode: engine.PulloutNotIn, + otherSideName: "id", + }, + { + sql: "select id from t where col1 = (select uid from t2 order by uid desc limit 1)", + opCode: engine.PulloutValue, + otherSideName: "col1", + }, + { + sql: "select id from t where exists (select uid from t2 where uid = 42)", + opCode: engine.PulloutExists, + otherSideName: "", + }, + } + + for i, tc := range tcs { + t.Run(fmt.Sprintf("%d_%s", i+1, tc.sql), func(t *testing.T) { + stmt, semTable := parseAndAnalyze(t, tc.sql, "d") + sel, _ := stmt.(*sqlparser.Select) + + var subq *sqlparser.Subquery + switch whereExpr := sel.Where.Expr.(type) { + case *sqlparser.ComparisonExpr: + subq = whereExpr.Right.(*sqlparser.Subquery) + case *sqlparser.ExistsExpr: + subq = whereExpr.Subquery + } + + extractedSubq := semTable.SubqueryRef[subq] + assert.True(t, sqlparser.EqualsExpr(&sqlparser.Subquery{Select: extractedSubq.Subquery}, subq)) + assert.True(t, sqlparser.EqualsExpr(extractedSubq.Original, sel.Where.Expr)) + assert.EqualValues(t, tc.opCode, extractedSubq.OpCode) + if tc.otherSideName == "" { + assert.Nil(t, extractedSubq.OtherSide) + } else { + assert.True(t, sqlparser.EqualsExpr(extractedSubq.OtherSide, sqlparser.NewColName(tc.otherSideName))) + } + }) + } +} + +func TestSubqueriesMappingSelectExprs(t *testing.T) { + tcs := []struct { + sql string + selExprIdx int + }{ + { + sql: "select (select id from t1)", + selExprIdx: 0, + }, + { + sql: "select id, (select id from t1) from t1", + selExprIdx: 1, + }, + } + + for i, tc := range tcs { + t.Run(fmt.Sprintf("%d_%s", i+1, tc.sql), func(t *testing.T) { + stmt, semTable := parseAndAnalyze(t, tc.sql, "d") + sel, _ := stmt.(*sqlparser.Select) + + subq := sel.SelectExprs[tc.selExprIdx].(*sqlparser.AliasedExpr).Expr.(*sqlparser.Subquery) + extractedSubq := semTable.SubqueryRef[subq] + assert.True(t, sqlparser.EqualsExpr(&sqlparser.Subquery{Select: extractedSubq.Subquery}, subq)) + assert.True(t, sqlparser.EqualsExpr(extractedSubq.Original, subq)) + assert.EqualValues(t, engine.PulloutValue, extractedSubq.OpCode) + }) + } +} + func TestSubqueryOrderByBinding(t *testing.T) { queries := []struct { query string From 986cb67c75f23aac9d6a35dd5518211a6899cfb8 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 11:14:06 +0200 Subject: [PATCH 17/19] allow any ComparisonOperator for comparison subqueries Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/rewrite.go | 6 ------ go/vt/vtgate/planbuilder/rewrite_test.go | 6 +++--- go/vt/vtgate/semantics/analyzer_test.go | 5 +++++ 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index 830f38ed60f..cc913f0b007 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -107,12 +107,6 @@ func (r *rewriter) rewriteUp(cursor *sqlparser.Cursor) bool { } func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.ComparisonExpr) error { - if node.Operator != sqlparser.InOp && - node.Operator != sqlparser.NotInOp && - node.Operator != sqlparser.EqualOp && - node.Operator != sqlparser.NotEqualOp { - return nil - } subq, exp := semantics.GetSubqueryAndOtherSide(node) if subq == nil || exp == nil { return nil diff --git a/go/vt/vtgate/planbuilder/rewrite_test.go b/go/vt/vtgate/planbuilder/rewrite_test.go index a239a36bdd6..31f39323fed 100644 --- a/go/vt/vtgate/planbuilder/rewrite_test.go +++ b/go/vt/vtgate/planbuilder/rewrite_test.go @@ -41,7 +41,7 @@ func TestSubqueryRewrite(t *testing.T) { output: "select 1 from t1 where :__sq_has_values1", }, { input: "select id from t1 where id in (select 1)", - output: "select id from t1 where :__sq_has_values1 = 1 and id in ::__sq1", + output: "select id from t1 where (:__sq_has_values1 = 1 and id in ::__sq1)", }, { input: "select id from t1 where id not in (select 1)", output: "select id from t1 where :__sq_has_values1 = 0 or id not in ::__sq1", @@ -65,7 +65,7 @@ func TestSubqueryRewrite(t *testing.T) { output: "select id from t1 where not :__sq_has_values1 and :__sq_has_values2", }, { input: "select (select 1), (select 2) from t1 join t2 on t1.id = (select 1) where t1.id in (select 1)", - output: "select :__sq2, :__sq3 from t1 join t2 on t1.id = :__sq1 where :__sq_has_values4 = 1 and t1.id in ::__sq4", + output: "select :__sq2, :__sq3 from t1 join t2 on t1.id = :__sq1 where (:__sq_has_values4 = 1 and t1.id in ::__sq4)", }} for _, tcase := range tcases { t.Run(tcase.input, func(t *testing.T) { @@ -123,7 +123,7 @@ func TestHavingRewrite(t *testing.T) { output: "select count(*) as k from t1 where (x = 1 or y = 2) and a = 1 having count(*) = 1", }, { input: "select 1 from t1 where x in (select 1 from t2 having a = 1)", - output: "select 1 from t1 where :__sq_has_values1 = 1 and x in ::__sq1", + output: "select 1 from t1 where (:__sq_has_values1 = 1 and x in ::__sq1)", sqs: map[string]string{"__sq1": "select 1 from t2 where a = 1"}, }, {input: "select 1 from t1 group by a having a = 1 and count(*) > 1", output: "select 1 from t1 where a = 1 group by a having count(*) > 1", diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index a0218a81f79..c910c0b3aba 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -539,6 +539,11 @@ func TestSubqueriesMappingWhereClause(t *testing.T) { opCode: engine.PulloutExists, otherSideName: "", }, + { + sql: "select id from t where col1 >= (select uid from t2 where uid = 42)", + opCode: engine.PulloutValue, + otherSideName: "col1", + }, } for i, tc := range tcs { From cfa570549975263dd65c55e70b8796151ebfb899 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 12 Oct 2021 11:39:28 +0200 Subject: [PATCH 18/19] correction of a few nits and addition of a new plan test Signed-off-by: Florent Poinsard --- .../planbuilder/querytree_transformers.go | 8 +- go/vt/vtgate/planbuilder/rewrite.go | 18 +-- .../planbuilder/testdata/from_cases.txt | 121 ++++++++++++++++++ go/vt/vtgate/semantics/tabletset.go | 31 ----- go/vt/vtgate/semantics/tabletset_test.go | 8 -- 5 files changed, 134 insertions(+), 52 deletions(-) diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index 617da594bc8..e9759d2c6db 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -502,8 +502,8 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { } type subQReplacer struct { - exprToReplaceBySqExpr []*sqlparser.ExtractedSubquery - replaced bool + subqueryToReplace []*sqlparser.ExtractedSubquery + replaced bool } func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { @@ -511,7 +511,7 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { if !ok { return true } - for _, replaceByExpr := range sqr.exprToReplaceBySqExpr { + for _, replaceByExpr := range sqr.subqueryToReplace { // we are comparing the ArgNames in case the expressions have been cloned if ext.ArgName == replaceByExpr.ArgName { cursor.Replace(ext.Original) @@ -527,7 +527,7 @@ func replaceSubQuery(ctx *planningContext, sel *sqlparser.Select) { if len(extractedSubqueries) == 0 { return } - sqr := &subQReplacer{exprToReplaceBySqExpr: extractedSubqueries} + sqr := &subQReplacer{subqueryToReplace: extractedSubqueries} sqlparser.Rewrite(sel, sqr.replacer, nil) for sqr.replaced { // to handle subqueries inside subqueries, we need to do this again and again until no replacements are left diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index cc913f0b007..90c66333434 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -25,10 +25,10 @@ import ( ) type rewriter struct { - semTable *semantics.SemTable - reservedVars *sqlparser.ReservedVars - subqueryStack []*sqlparser.ExtractedSubquery - err error + semTable *semantics.SemTable + reservedVars *sqlparser.ReservedVars + inSubquery int + err error } func queryRewrite(semTable *semantics.SemTable, reservedVars *sqlparser.ReservedVars, statement sqlparser.SelectStatement) error { @@ -58,7 +58,7 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { case *sqlparser.AliasedTableExpr: // rewrite names of the routed tables for the subquery // We only need to do this for non-derived tables and if they are in a subquery - if _, isDerived := node.Expr.(*sqlparser.DerivedTable); isDerived || len(r.subqueryStack) == 0 { + if _, isDerived := node.Expr.(*sqlparser.DerivedTable); isDerived || r.inSubquery == 0 { break } // find the tableSet and tableInfo that this table points to @@ -101,7 +101,7 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { func (r *rewriter) rewriteUp(cursor *sqlparser.Cursor) bool { switch cursor.Node().(type) { case *sqlparser.Subquery: - r.subqueryStack = r.subqueryStack[:len(r.subqueryStack)-1] + r.inSubquery-- } return r.err == nil } @@ -117,7 +117,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } - r.subqueryStack = append(r.subqueryStack, semTableSQ) + r.inSubquery++ argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName semTableSQ.HasValuesArg = hasValuesArg @@ -133,7 +133,7 @@ func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subq if semTableSQ.ArgName != "" || engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { return nil } - r.subqueryStack = append(r.subqueryStack, semTableSQ) + r.inSubquery++ argName := r.reservedVars.ReserveSubQuery() semTableSQ.ArgName = argName cursor.Replace(semTableSQ) @@ -146,7 +146,7 @@ func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlpars return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "BUG: came across subquery that was not in the subq map") } - r.subqueryStack = append(r.subqueryStack, semTableSQ) + r.inSubquery++ argName := r.reservedVars.ReserveHasValuesSubQuery() semTableSQ.ArgName = argName cursor.Replace(semTableSQ) diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index b477e8d0933..c7435ecd818 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt @@ -4273,3 +4273,124 @@ Gen4 plan same as above ] } } + +# two subqueries with different Select and OpCode +"select id from user where id in (select id from user_extra) and col = (select user_id from user_extra limit 1)" +{ + "QueryType": "SELECT", + "Original": "select id from user where id in (select id from user_extra) and col = (select user_id from user_extra limit 1)", + "Instructions": { + "OperatorType": "Subquery", + "Variant": "PulloutIn", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from user_extra where 1 != 1", + "Query": "select id from user_extra", + "Table": "user_extra" + }, + { + "OperatorType": "Subquery", + "Variant": "PulloutValue", + "Inputs": [ + { + "OperatorType": "Limit", + "Count": 1, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select user_id from user_extra where 1 != 1", + "Query": "select user_id from user_extra limit :__upper_limit", + "Table": "user_extra" + } + ] + }, + { + "OperatorType": "Route", + "Variant": "SelectIN", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from `user` where 1 != 1", + "Query": "select id from `user` where col = :__sq1 and :__sq_has_values2 = 1 and id in ::__vals", + "Table": "`user`", + "Values": [ + "::__sq2" + ], + "Vindex": "user_index" + } + ] + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select id from user where id in (select id from user_extra) and col = (select user_id from user_extra limit 1)", + "Instructions": { + "OperatorType": "Subquery", + "Variant": "PulloutValue", + "Inputs": [ + { + "OperatorType": "Limit", + "Count": 1, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select user_id from user_extra where 1 != 1", + "Query": "select user_id from user_extra limit :__upper_limit", + "Table": "user_extra" + } + ] + }, + { + "OperatorType": "Subquery", + "Variant": "PulloutIn", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from user_extra where 1 != 1", + "Query": "select id from user_extra", + "Table": "user_extra" + }, + { + "OperatorType": "Route", + "Variant": "SelectIN", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from `user` where 1 != 1", + "Query": "select id from `user` where (:__sq_has_values1 = 1 and id in ::__vals) and col = :__sq2", + "Table": "`user`", + "Values": [ + "::__sq1" + ], + "Vindex": "user_index" + } + ] + } + ] + } +} diff --git a/go/vt/vtgate/semantics/tabletset.go b/go/vt/vtgate/semantics/tabletset.go index 813a953fd11..54aab332ebc 100644 --- a/go/vt/vtgate/semantics/tabletset.go +++ b/go/vt/vtgate/semantics/tabletset.go @@ -97,13 +97,6 @@ func (ts *largeTableSet) mergeSmall(small uint64) *largeTableSet { return &largeTableSet{merged} } -func (ts *largeTableSet) removeSmall(small uint64) *largeTableSet { - merged := make([]uint64, len(ts.tables)) - copy(merged, ts.tables) - merged[0] &= ^small - return &largeTableSet{merged} -} - func (ts *largeTableSet) mergeInPlace(other *largeTableSet) { if len(other.tables) > len(ts.tables) { merged := make([]uint64, len(other.tables)) @@ -115,20 +108,10 @@ func (ts *largeTableSet) mergeInPlace(other *largeTableSet) { } } -func (ts *largeTableSet) removeInPlace(other *largeTableSet) { - for i := range ts.tables { - ts.tables[i] &= ^other.tables[i] - } -} - func (ts *largeTableSet) mergeSmallInPlace(small uint64) { ts.tables[0] |= small } -func (ts *largeTableSet) removeSmallInPlace(small uint64) { - ts.tables[0] &= ^small -} - func (ts *largeTableSet) tableOffset() (offset int) { var found bool for chunk, t := range ts.tables { @@ -300,20 +283,6 @@ func (ts *TableSet) MergeInPlace(other TableSet) { } } -// RemoveInPlace removes all the tables in `other` from this TableSet -func (ts *TableSet) RemoveInPlace(other TableSet) { - switch { - case ts.large == nil && other.large == nil: - ts.small &= ^other.small - case ts.large == nil: - ts.large = other.large.removeSmall(ts.small) - case other.large == nil: - ts.large.removeSmallInPlace(other.small) - default: - ts.large.removeInPlace(other.large) - } -} - // AddTable adds the given table to this set func (ts *TableSet) AddTable(tableidx int) { switch { diff --git a/go/vt/vtgate/semantics/tabletset_test.go b/go/vt/vtgate/semantics/tabletset_test.go index 0562c3278a2..c901f23974e 100644 --- a/go/vt/vtgate/semantics/tabletset_test.go +++ b/go/vt/vtgate/semantics/tabletset_test.go @@ -77,14 +77,6 @@ func TestTableSet_LargeTablesConstituents(t *testing.T) { assert.Equal(t, expected, ts.Constituents()) } -func TestRemoveSmall(t *testing.T) { - ts1 := SingleTableSet(1) - ts2 := SingleTableSet(2) - tsX := ts1.Merge(ts2) - tsX.RemoveInPlace(ts2) - assert.Equal(t, tsX.Constituents(), ts1.Constituents()) -} - func TestTabletSet_LargeMergeInPlace(t *testing.T) { const SetRange = 256 const Blocks = 64 From 24ad27c242074dfb8692dc9519f0ceae00ed60d9 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 13 Oct 2021 00:06:00 +0530 Subject: [PATCH 19/19] added pullout vars to the plan description Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/pullout_subquery.go | 12 + .../ddl_cases_no_default_keyspace.txt | 13 +- .../planbuilder/testdata/filter_cases.txt | 149 +++++++++- .../planbuilder/testdata/from_cases.txt | 258 +++++------------- .../testdata/postprocess_cases.txt | 41 ++- .../planbuilder/testdata/select_cases.txt | 80 +++++- .../planbuilder/testdata/tpch_cases.txt | 16 ++ .../testdata/unsupported_cases.txt | 1 - .../planbuilder/testdata/wireup_cases.txt | 16 +- 9 files changed, 376 insertions(+), 210 deletions(-) diff --git a/go/vt/vtgate/engine/pullout_subquery.go b/go/vt/vtgate/engine/pullout_subquery.go index 2378ea50a6d..8f51f35e810 100644 --- a/go/vt/vtgate/engine/pullout_subquery.go +++ b/go/vt/vtgate/engine/pullout_subquery.go @@ -171,9 +171,21 @@ func (ps *PulloutSubquery) execSubquery(vcursor VCursor, bindVars map[string]*qu } func (ps *PulloutSubquery) description() PrimitiveDescription { + other := map[string]interface{}{} + var pulloutVars []string + if ps.HasValues != "" { + pulloutVars = append(pulloutVars, ps.HasValues) + } + if ps.SubqueryResult != "" { + pulloutVars = append(pulloutVars, ps.SubqueryResult) + } + if len(pulloutVars) > 0 { + other["PulloutVars"] = pulloutVars + } return PrimitiveDescription{ OperatorType: "Subquery", Variant: ps.Opcode.String(), + Other: other, } } diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt index 335a3b62fb0..18cb418f74a 100644 --- a/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt +++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.txt @@ -509,18 +509,7 @@ Gen4 plan same as above "Query": "create view view_a as select * from music where user_id = 1" } } -{ - "QueryType": "DDL", - "Original": "create view user.view_a as select sql_calc_found_rows * from music where user_id = 1", - "Instructions": { - "OperatorType": "DDL", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "Query": "create view view_a as select * from music where user_id = 1" - } -} +Gen4 plan same as above # DDL "create index a on user(id)" diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index 61339d7a816..33d559d716b 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1704,6 +1704,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1740,6 +1744,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1779,6 +1787,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutNotIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1805,12 +1817,20 @@ Gen4 plan same as above ] } } +Gen4 plan same as above + +# cross-shard subquery in EXISTS clause. +"select id from user where exists (select col from user)" { "QueryType": "SELECT", - "Original": "select id from user where id not in (select col from user)", + "Original": "select id from user where exists (select col from user)", "Instructions": { "OperatorType": "Subquery", - "Variant": "PulloutNotIn", + "Variant": "PulloutExists", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1831,21 +1851,21 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where :__sq_has_values1 = 0 or id not in ::__sq1", + "Query": "select id from `user` where :__sq_has_values1", "Table": "`user`" } ] } } - -# cross-shard subquery in EXISTS clause. -"select id from user where exists (select col from user)" { "QueryType": "SELECT", "Original": "select id from user where exists (select col from user)", "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutExists", + "PulloutVars": [ + "__sq_has_values1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1872,7 +1892,6 @@ Gen4 plan same as above ] } } -Gen4 plan same as above # cross-shard subquery as expression "select id from user where id = (select col from user)" @@ -1882,6 +1901,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1922,10 +1945,18 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1975,10 +2006,18 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Route", @@ -2050,6 +2089,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2310,6 +2353,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Route", @@ -2329,6 +2376,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2371,6 +2422,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Route", @@ -2390,6 +2445,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2518,6 +2577,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2554,6 +2617,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2724,6 +2791,49 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutExists", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectEqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u2.`name` from `user` as u2 where 1 != 1", + "Query": "select u2.`name` from `user` as u2 where u2.id = 5", + "Table": "`user`", + "Values": [ + 5 + ], + "Vindex": "user_index" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u1.col from `user` as u1 where 1 != 1", + "Query": "select u1.col from `user` as u1 where not :__sq_has_values1", + "Table": "`user`" + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select u1.col from user as u1 where not exists (select u2.name from user u2 where u2.id = 5)", + "Instructions": { + "OperatorType": "Subquery", + "Variant": "PulloutExists", + "PulloutVars": [ + "__sq_has_values1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2754,7 +2864,6 @@ Gen4 plan same as above ] } } -Gen4 plan same as above # The outer and first inner are SelectEqualUnique with same Vindex value, the second inner has different Vindex value "select id from user where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 5) and id in (select user_extra.col from user_extra where user_extra.user_id = 4)" @@ -2764,6 +2873,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2804,6 +2917,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Route", @@ -2847,6 +2964,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2887,6 +3008,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3199,6 +3324,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3239,6 +3368,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index c7435ecd818..aab055e22e7 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt @@ -106,6 +106,10 @@ Gen4 error: Incorrect usage/placement of 'NEXT' "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -146,6 +150,10 @@ Gen4 error: Incorrect usage/placement of 'NEXT' "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -722,64 +730,7 @@ Gen4 plan same as above ] } } -{ - "QueryType": "SELECT", - "Original": "select user.col from user left join user_extra as e left join unsharded as m1 on m1.col = e.col on user.col = e.col", - "Instructions": { - "OperatorType": "Join", - "Variant": "LeftJoin", - "JoinColumnIndexes": "-1", - "JoinVars": { - "user_col": 0 - }, - "TableName": "`user`_user_extra_unsharded", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select `user`.col from `user` where 1 != 1", - "Query": "select `user`.col from `user`", - "Table": "`user`" - }, - { - "OperatorType": "Join", - "Variant": "LeftJoin", - "JoinVars": { - "e_col": 0 - }, - "TableName": "user_extra_unsharded", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select e.col from user_extra as e where 1 != 1", - "Query": "select e.col from user_extra as e where e.col = :user_col", - "Table": "user_extra" - }, - { - "OperatorType": "Route", - "Variant": "SelectUnsharded", - "Keyspace": { - "Name": "main", - "Sharded": false - }, - "FieldQuery": "select 1 from unsharded as m1 where 1 != 1", - "Query": "select 1 from unsharded as m1 where m1.col = :e_col", - "Table": "unsharded" - } - ] - } - ] - } -} +Gen4 plan same as above # Right join "select m1.col from unsharded as m1 right join unsharded as m2 on m1.a=m2.b" @@ -2404,72 +2355,7 @@ Gen4 plan same as above ] } } -{ - "QueryType": "SELECT", - "Original": "select t.col1 from (select user.id, user.col1 from user join user_extra) as t join unsharded on unsharded.col1 = t.col1 and unsharded.id = t.id", - "Instructions": { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "-1", - "JoinVars": { - "t_col1": 0, - "t_id": 1 - }, - "TableName": "`user`_user_extra_unsharded", - "Inputs": [ - { - "OperatorType": "SimpleProjection", - "Columns": [ - 1, - 0 - ], - "Inputs": [ - { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "-1,-2", - "TableName": "`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "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": "SelectScatter", - "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": "SelectUnsharded", - "Keyspace": { - "Name": "main", - "Sharded": false - }, - "FieldQuery": "select 1 from unsharded where 1 != 1", - "Query": "select 1 from unsharded where unsharded.col1 = :t_col1 and unsharded.id = :t_id", - "Table": "unsharded" - } - ] - } -} +Gen4 plan same as above # wire-up on within cross-shard derived table "select t.id from (select user.id, user.col1 from user join user_extra on user_extra.col = user.col) as t" @@ -2627,67 +2513,7 @@ Gen4 plan same as above ] } } -{ - "QueryType": "SELECT", - "Original": "select t.col1 from unsharded_a ua join (select user.id, user.col1 from user join user_extra) as t", - "Instructions": { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "1", - "TableName": "unsharded_a_`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectUnsharded", - "Keyspace": { - "Name": "main", - "Sharded": false - }, - "FieldQuery": "select 1 from unsharded_a as ua where 1 != 1", - "Query": "select 1 from unsharded_a as ua", - "Table": "unsharded_a" - }, - { - "OperatorType": "SimpleProjection", - "Columns": [ - 1 - ], - "Inputs": [ - { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "-1,-2", - "TableName": "`user`_user_extra", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "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": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select 1 from user_extra where 1 != 1", - "Query": "select 1 from user_extra", - "Table": "user_extra" - } - ] - } - ] - } - ] - } -} +Gen4 plan same as above # Join with cross-shard derived table on rhs - push down join predicate to derived table "select t.col1 from unsharded_a ua join (select user.id, user.col1 from user join user_extra) as t on t.id = ua.id" @@ -2769,6 +2595,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2801,6 +2631,9 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2836,6 +2669,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2868,6 +2705,9 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2903,6 +2743,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2935,6 +2779,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -2970,6 +2818,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3021,6 +2873,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3093,6 +2949,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3127,6 +2987,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3188,6 +3052,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -3252,6 +3120,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -4282,6 +4154,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Route", @@ -4297,6 +4173,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Limit", @@ -4341,6 +4221,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values2", + "__sq2" + ], "Inputs": [ { "OperatorType": "Limit", @@ -4362,6 +4246,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index a674a78de97..eea474e0c09 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt @@ -187,6 +187,10 @@ Gen4 error: Column 'col1' in field list is ambiguous "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -223,6 +227,10 @@ Gen4 error: Column 'col1' in field list is ambiguous "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -549,6 +557,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -583,6 +595,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -901,6 +917,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -933,6 +953,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1072,7 +1096,6 @@ Gen4 plan same as above } } - # ORDER BY RAND() after pull-out subquery "select col from user where col in (select col2 from user) order by rand()" { @@ -1081,6 +1104,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1113,6 +1140,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1631,6 +1662,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1669,6 +1704,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index 4e16023506b..92669e331d5 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1199,6 +1199,45 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col from `user` where 1 != 1", + "Query": "select col from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select a, :__sq1 from unsharded where 1 != 1", + "Query": "select a, :__sq1 from unsharded", + "Table": "unsharded" + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select a, (select col from user) from unsharded", + "Instructions": { + "OperatorType": "Subquery", + "Variant": "PulloutValue", + "PulloutVars": [ + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1225,7 +1264,6 @@ Gen4 plan same as above ] } } -Gen4 plan same as above # sub-expression subquery in select "select a, 1+(select col from user) from unsharded" @@ -1235,6 +1273,45 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col from `user` where 1 != 1", + "Query": "select col from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectUnsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select a, 1 + :__sq1 from unsharded where 1 != 1", + "Query": "select a, 1 + :__sq1 from unsharded", + "Table": "unsharded" + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select a, 1+(select col from user) from unsharded", + "Instructions": { + "OperatorType": "Subquery", + "Variant": "PulloutValue", + "PulloutVars": [ + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1261,7 +1338,6 @@ Gen4 plan same as above ] } } -Gen4 plan same as above # select * from derived table expands specific columns "select * from (select user.id id1, user_extra.id id2 from user join user_extra) as t" diff --git a/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt b/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt index 2cef6bd4d3c..200efb1c916 100644 --- a/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/tpch_cases.txt @@ -562,6 +562,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Aggregate", @@ -629,6 +633,10 @@ Gen4 plan same as above "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Aggregate", @@ -686,6 +694,10 @@ Gen4 plan same as above { "OperatorType": "Subquery", "Variant": "PulloutNotIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -768,6 +780,10 @@ Gen4 error: unsupported: cross-shard correlated subquery { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index 26acd3b4190..08feb0c6598 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -361,7 +361,6 @@ Gen4 plan same as above "unsupported: where clause for vindex function must be of the form id = (where clause missing)" Gen4 plan same as above - "select func(keyspace_id) from user_index where id = :id" "unsupported: expression on results of a vindex function" Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt index b9cc301eb71..d0e232c2184 100644 --- a/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt @@ -79,7 +79,6 @@ } } - # bind var already in use "select e.col, u.id uid, e.id eid from user u join user_extra e having uid = eid and e.col = :uid" { @@ -1073,6 +1072,10 @@ "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Limit", @@ -1137,6 +1140,10 @@ "Instructions": { "OperatorType": "Subquery", "Variant": "PulloutIn", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Limit", @@ -1208,6 +1215,10 @@ { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq_has_values1", + "__sq1" + ], "Inputs": [ { "OperatorType": "Route", @@ -1268,6 +1279,9 @@ { "OperatorType": "Subquery", "Variant": "PulloutValue", + "PulloutVars": [ + "__sq1" + ], "Inputs": [ { "OperatorType": "Route",