From 53582c0ff80029d583f4577fc3387ad5cf45556d Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 27 Sep 2021 10:33:10 +0200 Subject: [PATCH 01/10] addition of failing E2E hasValues test Signed-off-by: Florent Poinsard --- go/test/endtoend/vtgate/misc_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 66db6549c95..897b1ad6990 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -753,6 +753,19 @@ func TestSimpleOrderBy(t *testing.T) { assertMatches(t, conn, `SELECT id2 FROM t1 ORDER BY id2 ASC`, `[[INT64(5)] [INT64(6)] [INT64(7)] [INT64(8)] [INT64(9)] [INT64(10)]]`) } +func TestSubqueriesHasValues(t *testing.T) { + defer cluster.PanicHandler(t) + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + defer exec(t, conn, `delete from t1`) + exec(t, conn, "insert into t1(id1, id2) values (0,1),(1,2),(2,3),(3,4),(4,5),(5,6)") + assertMatches(t, conn, `SELECT id2 FROM t1 WHERE id1 IN (SELECT id1 FROM t1 WHERE id1 > 10)`, `[]`) + assertMatches(t, conn, `SELECT id2 FROM t1 WHERE id1 NOT IN (SELECT id1 FROM t1 WHERE id1 > 10)`, `[[INT64(1)] [INT64(5)] [INT64(2)] [INT64(3)] [INT64(4)] [INT64(6)]]`) +} + func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) { t.Helper() qr := exec(t, conn, query) From 02131ed287c5da1102f3b0be88071e9b642780dc Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 27 Sep 2021 13:49:54 +0200 Subject: [PATCH 02/10] Support for HasValues in Subquery pullout values Signed-off-by: Florent Poinsard --- go/vt/sqlparser/analyzer.go | 31 +++++++++- go/vt/sqlparser/ast_rewriting.go | 16 ++++++ go/vt/vtgate/planbuilder/abstract/operator.go | 1 + go/vt/vtgate/planbuilder/abstract/subquery.go | 1 + .../planbuilder/querytree_transformers.go | 2 +- go/vt/vtgate/planbuilder/rewrite.go | 56 +++++++++++++++++-- go/vt/vtgate/planbuilder/route_planning.go | 9 +-- go/vt/vtgate/planbuilder/subquerytree.go | 22 ++++---- go/vt/vtgate/planbuilder/testdata/onecase.txt | 18 ++++++ go/vt/vtgate/semantics/semantic_state.go | 7 ++- 10 files changed, 138 insertions(+), 25 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 61140710828..0775fa7668c 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -326,7 +326,7 @@ func SplitAndExpression(filters []Expr, node Expr) []Expr { return append(filters, node) } -// AndExpressions ands together two expression, minimising the expr when possible +// AndExpressions ands together two or more expressions, minimising the expr when possible func AndExpressions(exprs ...Expr) Expr { switch len(exprs) { case 0: @@ -355,6 +355,35 @@ func AndExpressions(exprs ...Expr) Expr { } } +// OrExpressions ors together two or more expressions, minimising the expr when possible +func OrExpressions(exprs ...Expr) Expr { + switch len(exprs) { + case 0: + return nil + case 1: + return exprs[0] + default: + result := (Expr)(nil) + for i, expr := range exprs { + if result == nil { + result = expr + } else { + found := false + for j := 0; j < i; j++ { + if EqualsExpr(expr, exprs[j]) { + found = true + break + } + } + if !found { + result = &OrExpr{Left: result, Right: expr} + } + } + } + return result + } +} + // TableFromStatement returns the qualified table name for the query. // This works only for select statements. func TableFromStatement(sql string) (TableName, error) { diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 0f44cca4531..a35e1eaf9e5 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -101,6 +101,22 @@ func (r *ReservedVars) ReserveSubQuery() string { } } +// ReserveSubQueryWithHasValues returns the next argument name to replace subquery with pullout value. +func (r *ReservedVars) ReserveSubQueryWithHasValues() (string, string) { + for { + r.sqNext++ + joinVar := strconv.AppendInt(subQueryBaseArgName, r.sqNext, 10) + hasValuesJoinVar := strconv.AppendInt(HasValueSubQueryBaseName, r.sqNext, 10) + _, joinVarOK := r.reserved[string(joinVar)] + _, hasValuesJoinVarOK := r.reserved[string(hasValuesJoinVar)] + if !joinVarOK && !hasValuesJoinVarOK { + r.reserved[string(joinVar)] = struct{}{} + r.reserved[string(hasValuesJoinVar)] = struct{}{} + return string(joinVar), string(hasValuesJoinVar) + } + } +} + // ReserveHasValuesSubQuery returns the next argument name to replace subquery with has value. func (r *ReservedVars) ReserveHasValuesSubQuery() string { for { diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index 0f6f5a6d79f..7f97940f4b4 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -215,6 +215,7 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl Inner: opInner, Type: sq.OpCode, ArgName: sq.ArgName, + HasValues: sq.HasValues, }) } } diff --git a/go/vt/vtgate/planbuilder/abstract/subquery.go b/go/vt/vtgate/planbuilder/abstract/subquery.go index 4e1b3e31fd5..d95f6cfb16c 100644 --- a/go/vt/vtgate/planbuilder/abstract/subquery.go +++ b/go/vt/vtgate/planbuilder/abstract/subquery.go @@ -38,6 +38,7 @@ type SubQueryInner struct { Type engine.PulloutOpcode SelectStatement *sqlparser.Select ArgName string + HasValues string } // TableID implements the Operator interface diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index 88a503a9b67..fda2b6b4799 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -98,7 +98,7 @@ func transformSubqueryTree(ctx *planningContext, n *subqueryTree) (logicalPlan, return nil, err } - plan := newPulloutSubquery(n.opcode, n.argName, "", innerPlan) + plan := newPulloutSubquery(n.opcode, n.argName, n.hasValues, innerPlan) outerPlan, err := transformToLogicalPlan(ctx, n.outer) if err != nil { return nil, err diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index bf787d8575a..2d9eccb443d 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -41,6 +41,8 @@ func (r *rewriter) rewriteDown(cursor *sqlparser.Cursor) bool { switch node := cursor.Node().(type) { case *sqlparser.Select: rewriteHavingClause(node) + case *sqlparser.ComparisonExpr: + rewriteInSubquery(cursor, r, node) case *sqlparser.ExistsExpr: return r.rewriteExistsSubquery(cursor, node) case *sqlparser.Subquery: @@ -92,20 +94,62 @@ func (r *rewriter) rewriteUp(cursor *sqlparser.Cursor) bool { return true } -func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) { - semTableSQ, found := r.semTable.SubqueryRef[node] +func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.ComparisonExpr) { + if node.Operator != sqlparser.InOp && node.Operator != sqlparser.NotInOp { + return + } + 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 + } else { + return + } + + semTableSQ, found := r.semTable.SubqueryRef[subq] if !found { // should never happen return } - argName := r.reservedVars.ReserveSubQuery() + argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName + semTableSQ.HasValues = hasValuesArg + 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)) + } +} + +func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) { + semTableSQ, found := r.semTable.SubqueryRef[node] + if !found { + // should never happen + return + } switch semTableSQ.OpCode { - case engine.PulloutIn, engine.PulloutNotIn: - cursor.Replace(sqlparser.NewListArg(argName)) - default: + case engine.PulloutValue: + argName := r.reservedVars.ReserveSubQuery() + semTableSQ.ArgName = argName cursor.Replace(sqlparser.NewArgument(argName)) } } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index dc1e85b4f69..b73fc93bc8a 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -159,10 +159,11 @@ 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, - inner: treeInner, - opcode: inner.Type, - argName: inner.ArgName, + subquery: inner.SelectStatement, + inner: treeInner, + opcode: inner.Type, + argName: inner.ArgName, + hasValues: inner.HasValues, }) } else { outerTree = merged diff --git a/go/vt/vtgate/planbuilder/subquerytree.go b/go/vt/vtgate/planbuilder/subquerytree.go index 328c336c324..2e2cbe4365e 100644 --- a/go/vt/vtgate/planbuilder/subquerytree.go +++ b/go/vt/vtgate/planbuilder/subquerytree.go @@ -25,11 +25,12 @@ import ( ) type subqueryTree struct { - subquery *sqlparser.Select - outer queryTree - inner queryTree - opcode engine.PulloutOpcode - argName string + subquery *sqlparser.Select + outer queryTree + inner queryTree + opcode engine.PulloutOpcode + argName string + hasValues string } var _ queryTree = (*subqueryTree)(nil) @@ -44,11 +45,12 @@ 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, + subquery: s.subquery, + outer: s.outer.clone(), + inner: s.inner.clone(), + opcode: s.opcode, + argName: s.argName, + hasValues: s.hasValues, } return result } diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.txt b/go/vt/vtgate/planbuilder/testdata/onecase.txt index e819513f354..b79f246468b 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. +# Composite IN: RHS not tuple +"select id from user where (col1, name) in (select * from music where music.user_id=user.id)" +{ + "QueryType": "SELECT", + "Original": "select id from user where (col1, name) in (select * from music where music.user_id=user.id)", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from `user` where 1 != 1", + "Query": "select id from `user` where (col1, `name`) in (select * from music where music.user_id = `user`.id)", + "Table": "`user`" + } +} +Gen4 plan same as above diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 01a9f0734d1..190cabcc1ae 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -104,9 +104,10 @@ type ( } subquery struct { - ArgName string - SubQuery *sqlparser.Subquery - OpCode engine.PulloutOpcode + ArgName string + HasValues string + SubQuery *sqlparser.Subquery + OpCode engine.PulloutOpcode } // SchemaInformation is used tp provide table information from Vschema. From 086338ca3f569fb01334196cddd73ea49f6874aa Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 27 Sep 2021 15:18:29 +0200 Subject: [PATCH 03/10] Enabled the replacement of subquery arguments by a sqlparser.expr Signed-off-by: Florent Poinsard --- go/vt/sqlparser/analyzer.go | 15 +++++ go/vt/vtgate/planbuilder/abstract/operator.go | 2 + go/vt/vtgate/planbuilder/abstract/subquery.go | 2 + go/vt/vtgate/planbuilder/gen4_planner.go | 11 ++-- .../planbuilder/querytree_transformers.go | 57 ++++++++++++------- go/vt/vtgate/planbuilder/rewrite.go | 23 ++++---- go/vt/vtgate/planbuilder/route_planning.go | 11 +++- go/vt/vtgate/semantics/semantic_state.go | 2 + 8 files changed, 86 insertions(+), 37 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 0775fa7668c..3489626947a 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -355,6 +355,21 @@ func AndExpressions(exprs ...Expr) Expr { } } +// SplitOrExpression breaks up the Expr into OR-separated conditions +// and appends them to filters. Outer parenthesis are removed. Precedence +// should be taken into account if expressions are recombined. +func SplitOrExpression(filters []Expr, node Expr) []Expr { + if node == nil { + return filters + } + switch node := node.(type) { + case *OrExpr: + filters = SplitOrExpression(filters, node.Left) + return SplitOrExpression(filters, node.Right) + } + return append(filters, node) +} + // OrExpressions ors together two or more expressions, minimising the expr when possible func OrExpressions(exprs ...Expr) Expr { switch len(exprs) { diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index 7f97940f4b4..0a1e19b6379 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -216,6 +216,8 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl Type: sq.OpCode, ArgName: sq.ArgName, HasValues: sq.HasValues, + Replace: sq.Replace, + ReplaceBy: sq.ReplaceBy, }) } } diff --git a/go/vt/vtgate/planbuilder/abstract/subquery.go b/go/vt/vtgate/planbuilder/abstract/subquery.go index d95f6cfb16c..ea1b6c905ab 100644 --- a/go/vt/vtgate/planbuilder/abstract/subquery.go +++ b/go/vt/vtgate/planbuilder/abstract/subquery.go @@ -39,6 +39,8 @@ type SubQueryInner struct { SelectStatement *sqlparser.Select ArgName string HasValues string + Replace []sqlparser.Expr + ReplaceBy sqlparser.Expr } // TableID implements the Operator interface diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 0cbd64fb57c..caa3e38636e 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -178,10 +178,11 @@ 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, - sqToReplace: map[string]*sqlparser.Select{}, + reservedVars: reservedVars, + semTable: semTable, + vschema: vschema, + sqToReplace: map[string]*sqlparser.Select{}, + sqToReplaceExpr: map[sqlparser.Expr]sqlparser.Expr{}, } return ctx } @@ -216,7 +217,7 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat sel: node, } - replaceSubQuery(ctx.sqToReplace, node) + replaceSubQuery(ctx.sqToReplace, ctx.sqToReplaceExpr, 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 fda2b6b4799..3ab3fcbe1f9 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -324,7 +324,7 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { Comments: ctx.semTable.Comments, } - replaceSubQuery(ctx.sqToReplace, sel) + replaceSubQuery(ctx.sqToReplace, ctx.sqToReplaceExpr, sel) // TODO clean up when gen4 is the only planner var condition sqlparser.Expr @@ -439,39 +439,54 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { } type subQReplacer struct { - sqToReplace map[string]*sqlparser.Select - err error - replaced bool + sqToReplace map[string]*sqlparser.Select + sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr + err error + replaced bool } func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { - argName := argumentName(cursor.Node()) - if argName == "" { + 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: return true } - var node sqlparser.SQLNode - subqSelect, exists := sqr.sqToReplace[argName] - if !exists { - sqr.err = vterrors.Errorf(vtrpcpb.Code_INTERNAL, "[BUG] unable to find subquery with argument: %s", argName) - return false + var replaceBy sqlparser.Expr + for _, expr := range exprs { + found := false + for sqExprToReplace, replaceByExpr := range sqr.sqToReplaceExpr { + if sqlparser.EqualsExpr(expr, sqExprToReplace) { + found = true + replaceBy = replaceByExpr + break + } + } + if !found { + return false + } } - sq := &sqlparser.Subquery{Select: subqSelect} - node = sq - - // if the subquery is in an EXISTS, e.g. "__sq_has_values1" - // then we encapsulate the subquery in an exists expression. - if strings.HasPrefix(argName, string(sqlparser.HasValueSubQueryBaseName)) { - node = &sqlparser.ExistsExpr{Subquery: sq} + if replaceBy == nil { + return true } - cursor.Replace(node) + cursor.Replace(replaceBy) sqr.replaced = true return false } -func replaceSubQuery(sqToReplace map[string]*sqlparser.Select, sel *sqlparser.Select) { +func replaceSubQuery(sqToReplace map[string]*sqlparser.Select, expr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { if len(sqToReplace) > 0 { - sqr := &subQReplacer{sqToReplace: sqToReplace} + sqr := &subQReplacer{sqToReplace: sqToReplace, sqToReplaceExpr: expr} 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 2d9eccb443d..0e59a520a28 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -119,6 +119,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co argName, hasValuesArg := r.reservedVars.ReserveSubQueryWithHasValues() semTableSQ.ArgName = argName semTableSQ.HasValues = hasValuesArg + semTableSQ.ReplaceBy = node newSubQExpr := &sqlparser.ComparisonExpr{ Operator: node.Operator, Left: exp, @@ -137,21 +138,21 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co hasValuesExpr.Right = sqlparser.NewIntLiteral("0") cursor.Replace(sqlparser.OrExpressions(hasValuesExpr, newSubQExpr)) } + semTableSQ.Replace = append(semTableSQ.Replace, hasValuesExpr, newSubQExpr) } func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) { semTableSQ, found := r.semTable.SubqueryRef[node] - if !found { - // should never happen + if !found || semTableSQ.OpCode != engine.PulloutValue { return } - switch semTableSQ.OpCode { - case engine.PulloutValue: - argName := r.reservedVars.ReserveSubQuery() - semTableSQ.ArgName = argName - cursor.Replace(sqlparser.NewArgument(argName)) - } + argName := r.reservedVars.ReserveSubQuery() + arg := sqlparser.NewArgument(argName) + semTableSQ.ArgName = argName + semTableSQ.Replace = append(semTableSQ.Replace, arg) + semTableSQ.ReplaceBy = node + cursor.Replace(arg) } func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlparser.ExistsExpr) bool { @@ -162,9 +163,11 @@ func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlpars } argName := r.reservedVars.ReserveHasValuesSubQuery() + arg := sqlparser.NewArgument(argName) semTableSQ.ArgName = argName - - cursor.Replace(sqlparser.NewArgument(argName)) + semTableSQ.Replace = append(semTableSQ.Replace, arg) + semTableSQ.ReplaceBy = node + cursor.Replace(arg) return false } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index b73fc93bc8a..601d4af1977 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -39,7 +39,13 @@ type planningContext struct { semTable *semantics.SemTable vschema ContextVSchema // these helps in replacing the argNames with the subquery - sqToReplace map[string]*sqlparser.Select + sqToReplace map[string]*sqlparser.Select + sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr +} + +type subqueryReplaceContext struct { + exprs []sqlparser.Expr + sel *sqlparser.Select } func (c planningContext) isSubQueryToReplace(name string) bool { @@ -278,6 +284,9 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { ctx.sqToReplace[subq.ArgName] = subq.SelectStatement + for _, expr := range subq.Replace { + ctx.sqToReplaceExpr[expr] = subq.ReplaceBy + } // 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 diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 190cabcc1ae..38b67dd1100 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -108,6 +108,8 @@ type ( HasValues string SubQuery *sqlparser.Subquery OpCode engine.PulloutOpcode + Replace []sqlparser.Expr + ReplaceBy sqlparser.Expr } // SchemaInformation is used tp provide table information from Vschema. From f60acecd85e8a5c71ae7ec6bb6820e0acda7b91f Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 27 Sep 2021 17:40:42 +0200 Subject: [PATCH 04/10] Update expected output for plan tests and enhanced subquery variable transformation Signed-off-by: Florent Poinsard --- go/vt/sqlparser/analyzer.go | 3 +++ go/vt/vtgate/planbuilder/abstract/operator.go | 1 - .../planbuilder/querytree_transformers.go | 10 ++++++--- go/vt/vtgate/planbuilder/rewrite.go | 2 ++ .../planbuilder/testdata/filter_cases.txt | 22 +++++++++---------- .../planbuilder/testdata/from_cases.txt | 8 +++---- go/vt/vtgate/planbuilder/testdata/onecase.txt | 18 --------------- .../testdata/postprocess_cases.txt | 8 +++---- .../planbuilder/testdata/wireup_cases.txt | 2 +- 9 files changed, 32 insertions(+), 42 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 3489626947a..90cfc3972a2 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -336,6 +336,9 @@ func AndExpressions(exprs ...Expr) Expr { default: result := (Expr)(nil) for i, expr := range exprs { + if expr == nil { + continue + } if result == nil { result = expr } else { diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index 0a1e19b6379..e831b4ef299 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -34,7 +34,6 @@ type ( // * 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 - // * Distinct - Represents elimination of duplicates from the output of the input source Operator interface { // TableID returns a TableSet of the tables contained within TableID() semantics.TableSet diff --git a/go/vt/vtgate/planbuilder/querytree_transformers.go b/go/vt/vtgate/planbuilder/querytree_transformers.go index 3ab3fcbe1f9..59ec9543a01 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -463,23 +463,27 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { } var replaceBy sqlparser.Expr + var remainder sqlparser.Expr for _, expr := range exprs { found := false for sqExprToReplace, replaceByExpr := range sqr.sqToReplaceExpr { if sqlparser.EqualsExpr(expr, sqExprToReplace) { + allReplaceByExprs := sqlparser.SplitAndExpression(nil, replaceBy) + allReplaceByExprs = append(allReplaceByExprs, replaceByExpr) + replaceBy = sqlparser.AndExpressions(allReplaceByExprs...) found = true - replaceBy = replaceByExpr break } } if !found { - return false + remainder = sqlparser.AndExpressions(remainder, expr) } } if replaceBy == nil { return true } - cursor.Replace(replaceBy) + newNode := sqlparser.AndExpressions(remainder, replaceBy) + cursor.Replace(newNode) sqr.replaced = true return false } diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index 0e59a520a28..6247dc59801 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -139,6 +139,8 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co cursor.Replace(sqlparser.OrExpressions(hasValuesExpr, newSubQExpr)) } semTableSQ.Replace = append(semTableSQ.Replace, hasValuesExpr, newSubQExpr) + r.semTable.Recursive[hasValuesExpr] = r.semTable.RecursiveDeps(newSubQExpr) + r.semTable.Direct[hasValuesExpr] = r.semTable.DirectDeps(newSubQExpr) } func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subquery) { diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index d216a9c6487..f14b7164764 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -1289,7 +1289,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 (select m2 from `user` where `user`.id = u.id and `user`.col = :user_extra_col) and u.id in ::__vals", + "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)", "Table": "`user`", "Values": [ [ @@ -1380,7 +1380,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 (select m2 from `user` where `user`.id = u.id) and u.id in ::__vals", + "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)", "Table": "`user`", "Values": [ [ @@ -1462,7 +1462,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 (select m2 from `user` where `user`.id = 5) and u.id = 5", + "Query": "select u.m from `user` as u where u.id = 5 and u.id in (select m2 from `user` where `user`.id = 5)", "Table": "`user`", "Values": [ 5 @@ -1550,7 +1550,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 (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", + "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))", "Table": "`user`", "Values": [ [ @@ -1741,7 +1741,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where id in ::__vals", + "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", "Table": "`user`", "Values": [ "::__sq1" @@ -1812,7 +1812,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where id not in ::__sq1", + "Query": "select id from `user` where :__sq_has_values1 = 0 or id not in ::__sq1", "Table": "`user`" } ] @@ -1980,7 +1980,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id2 from `user` where 1 != 1", - "Query": "select id2 from `user` where id2 in ::__sq2", + "Query": "select id2 from `user` where :__sq_has_values2 = 1 and id2 in ::__sq2", "Table": "`user`" } ] @@ -2395,7 +2395,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where not id in ::__sq1 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" @@ -2555,7 +2555,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where id in ::__vals", + "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", "Table": "`user`", "Values": [ "::__sq1" @@ -2809,7 +2809,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 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 @@ -2892,7 +2892,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 ::__sq1 and id in (select user_extra.col from user_extra where user_extra.user_id = 5)", + "Query": "select id from `user` where id = 5 and not (:__sq_has_values1 = 1 and id in ::__sq1) and id in (select user_extra.col from user_extra where user_extra.user_id = 5)", "Table": "`user`", "Values": [ 5 diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt index f65a8a41fc8..467f9b7582a 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 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 `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 `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 `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/onecase.txt b/go/vt/vtgate/planbuilder/testdata/onecase.txt index b79f246468b..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. -# Composite IN: RHS not tuple -"select id from user where (col1, name) in (select * from music where music.user_id=user.id)" -{ - "QueryType": "SELECT", - "Original": "select id from user where (col1, name) in (select * from music where music.user_id=user.id)", - "Instructions": { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select id from `user` where 1 != 1", - "Query": "select id from `user` where (col1, `name`) in (select * from music where music.user_id = `user`.id)", - "Table": "`user`" - } -} -Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index aef29a91d9a..1973e46e862 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 id in ::__vals", + "Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals", "Table": "`user`", "Values": [ "::__sq1" @@ -664,7 +664,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 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`" } @@ -1013,7 +1013,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select col from `user` where 1 != 1", - "Query": "select col from `user` where 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`" } ] @@ -1670,7 +1670,7 @@ Gen4 plan same as above "Sharded": true }, "FieldQuery": "select col from `user` where 1 != 1", - "Query": "select col from `user` where 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/wireup_cases.txt b/go/vt/vtgate/planbuilder/testdata/wireup_cases.txt index 487b1d97583..708c1cb4ade 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 id in ::__vals", + "Query": "select 1 from `user` where :__sq_has_values1 = 1 and id in ::__vals", "Table": "`user`", "Values": [ "::__sq1" From 22e9169478b7f0a3f250c7433138b7ca004d85ae Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 27 Sep 2021 17:49:38 +0200 Subject: [PATCH 05/10] Enhanced code readability for subquery rewrite Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/abstract/operator.go | 2 +- go/vt/vtgate/planbuilder/gen4_planner.go | 4 ++-- go/vt/vtgate/planbuilder/querytree_transformers.go | 10 ++++------ go/vt/vtgate/planbuilder/rewrite.go | 8 ++++---- go/vt/vtgate/planbuilder/route_planning.go | 11 +++-------- go/vt/vtgate/planbuilder/routetree.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 12 ++++++------ 7 files changed, 21 insertions(+), 28 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index e831b4ef299..31b2618e7f4 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -215,7 +215,7 @@ func createOperatorFromSelect(sel *sqlparser.Select, semTable *semantics.SemTabl Type: sq.OpCode, ArgName: sq.ArgName, HasValues: sq.HasValues, - Replace: sq.Replace, + Replace: sq.ExprsNeedReplace, ReplaceBy: sq.ReplaceBy, }) } diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index caa3e38636e..541e212f94a 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -181,7 +181,7 @@ func newPlanningContext(reservedVars *sqlparser.ReservedVars, semTable *semantic reservedVars: reservedVars, semTable: semTable, vschema: vschema, - sqToReplace: map[string]*sqlparser.Select{}, + sqToReplaceArg: map[string]*sqlparser.Select{}, sqToReplaceExpr: map[sqlparser.Expr]sqlparser.Expr{}, } return ctx @@ -217,7 +217,7 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat sel: node, } - replaceSubQuery(ctx.sqToReplace, ctx.sqToReplaceExpr, node) + replaceSubQuery(ctx.sqToReplaceExpr, 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 59ec9543a01..eaa920cc78b 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -324,7 +324,7 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { Comments: ctx.semTable.Comments, } - replaceSubQuery(ctx.sqToReplace, ctx.sqToReplaceExpr, sel) + replaceSubQuery(ctx.sqToReplaceExpr, sel) // TODO clean up when gen4 is the only planner var condition sqlparser.Expr @@ -439,9 +439,7 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { } type subQReplacer struct { - sqToReplace map[string]*sqlparser.Select sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr - err error replaced bool } @@ -488,9 +486,9 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { return false } -func replaceSubQuery(sqToReplace map[string]*sqlparser.Select, expr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { - if len(sqToReplace) > 0 { - sqr := &subQReplacer{sqToReplace: sqToReplace, sqToReplaceExpr: expr} +func replaceSubQuery(sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { + if len(sqToReplaceExpr) > 0 { + sqr := &subQReplacer{sqToReplaceExpr: sqToReplaceExpr} 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 6247dc59801..d04ee6cb447 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -98,7 +98,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co if node.Operator != sqlparser.InOp && node.Operator != sqlparser.NotInOp { return } - subq := &sqlparser.Subquery{} + var subq *sqlparser.Subquery var exp sqlparser.Expr if lSubq, lIsSubq := node.Left.(*sqlparser.Subquery); lIsSubq { subq = lSubq @@ -138,7 +138,7 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co hasValuesExpr.Right = sqlparser.NewIntLiteral("0") cursor.Replace(sqlparser.OrExpressions(hasValuesExpr, newSubQExpr)) } - semTableSQ.Replace = append(semTableSQ.Replace, hasValuesExpr, newSubQExpr) + semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, hasValuesExpr, newSubQExpr) r.semTable.Recursive[hasValuesExpr] = r.semTable.RecursiveDeps(newSubQExpr) r.semTable.Direct[hasValuesExpr] = r.semTable.DirectDeps(newSubQExpr) } @@ -152,7 +152,7 @@ func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subq argName := r.reservedVars.ReserveSubQuery() arg := sqlparser.NewArgument(argName) semTableSQ.ArgName = argName - semTableSQ.Replace = append(semTableSQ.Replace, arg) + semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, arg) semTableSQ.ReplaceBy = node cursor.Replace(arg) } @@ -167,7 +167,7 @@ func (r *rewriter) rewriteExistsSubquery(cursor *sqlparser.Cursor, node *sqlpars argName := r.reservedVars.ReserveHasValuesSubQuery() arg := sqlparser.NewArgument(argName) semTableSQ.ArgName = argName - semTableSQ.Replace = append(semTableSQ.Replace, arg) + semTableSQ.ExprsNeedReplace = append(semTableSQ.ExprsNeedReplace, arg) semTableSQ.ReplaceBy = node cursor.Replace(arg) return false diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 601d4af1977..51c8e4d91a1 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -39,17 +39,12 @@ type planningContext struct { semTable *semantics.SemTable vschema ContextVSchema // these helps in replacing the argNames with the subquery - sqToReplace map[string]*sqlparser.Select + sqToReplaceArg map[string]*sqlparser.Select sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr } -type subqueryReplaceContext struct { - exprs []sqlparser.Expr - sel *sqlparser.Select -} - func (c planningContext) isSubQueryToReplace(name string) bool { - _, found := c.sqToReplace[name] + _, found := c.sqToReplaceArg[name] return found } @@ -283,7 +278,7 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre } func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { - ctx.sqToReplace[subq.ArgName] = subq.SelectStatement + ctx.sqToReplaceArg[subq.ArgName] = subq.SelectStatement for _, expr := range subq.Replace { ctx.sqToReplaceExpr[expr] = subq.ReplaceBy } diff --git a/go/vt/vtgate/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index 3004f8b9ef7..64dc143cf1c 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -409,7 +409,7 @@ func (rp *routeTree) planIsExpr(ctx *planningContext, node *sqlparser.IsExpr) (b } // makePlanValue transforms the given sqlparser.Expr into a sqltypes.PlanValue. -// If the given sqlparser.Expr is an argument and can be found in the rp.sqToReplace then the +// If the given sqlparser.Expr is an argument and can be found in the rp.sqToReplaceArg then the // method will stops and return nil values. // 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. diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 38b67dd1100..8421b66e982 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -104,12 +104,12 @@ type ( } subquery struct { - ArgName string - HasValues string - SubQuery *sqlparser.Subquery - OpCode engine.PulloutOpcode - Replace []sqlparser.Expr - ReplaceBy sqlparser.Expr + ArgName string + HasValues string + SubQuery *sqlparser.Subquery + OpCode engine.PulloutOpcode + ExprsNeedReplace []sqlparser.Expr + ReplaceBy sqlparser.Expr } // SchemaInformation is used tp provide table information from Vschema. From 44d9dd527c35ef152266685df26d846fd6eff3d2 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 28 Sep 2021 09:30:35 +0200 Subject: [PATCH 06/10] Updated expected results for rewrite unit tests Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/rewrite_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/planbuilder/rewrite_test.go b/go/vt/vtgate/planbuilder/rewrite_test.go index 40722b5b8de..cda6bf66f0b 100644 --- a/go/vt/vtgate/planbuilder/rewrite_test.go +++ b/go/vt/vtgate/planbuilder/rewrite_test.go @@ -41,10 +41,10 @@ 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 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 id not in ::__sq1", + output: "select id from t1 where :__sq_has_values1 = 0 or id not in ::__sq1", }, { input: "select id from t1 where id = (select 1)", output: "select id from t1 where id = :__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 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,10 +123,9 @@ 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 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", + }, {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", }} for _, tcase := range tcases { From 1a5e4ba1cdd24451aa221d3209ec50919438d7ce Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 28 Sep 2021 10:03:46 +0200 Subject: [PATCH 07/10] Improved code readability for subqueries Signed-off-by: Florent Poinsard --- go/vt/sqlparser/analyzer.go | 57 +++++++------------ go/vt/vtgate/planbuilder/abstract/operator.go | 14 ++--- go/vt/vtgate/planbuilder/abstract/subquery.go | 14 ++--- go/vt/vtgate/planbuilder/gen4_planner.go | 12 ++-- .../planbuilder/querytree_transformers.go | 14 ++--- go/vt/vtgate/planbuilder/rewrite.go | 27 ++++++--- go/vt/vtgate/planbuilder/route_planning.go | 15 ++--- go/vt/vtgate/planbuilder/routetree.go | 2 +- go/vt/vtgate/semantics/semantic_state.go | 11 ++-- 9 files changed, 83 insertions(+), 83 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 90cfc3972a2..5e479d281d5 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -326,38 +326,6 @@ func SplitAndExpression(filters []Expr, node Expr) []Expr { return append(filters, node) } -// AndExpressions ands together two or more expressions, minimising the expr when possible -func AndExpressions(exprs ...Expr) Expr { - switch len(exprs) { - case 0: - return nil - case 1: - return exprs[0] - default: - result := (Expr)(nil) - for i, expr := range exprs { - if expr == nil { - continue - } - if result == nil { - result = expr - } else { - found := false - for j := 0; j < i; j++ { - if EqualsExpr(expr, exprs[j]) { - found = true - break - } - } - if !found { - result = &AndExpr{Left: result, Right: expr} - } - } - } - return result - } -} - // SplitOrExpression breaks up the Expr into OR-separated conditions // and appends them to filters. Outer parenthesis are removed. Precedence // should be taken into account if expressions are recombined. @@ -373,8 +341,8 @@ func SplitOrExpression(filters []Expr, node Expr) []Expr { return append(filters, node) } -// OrExpressions ors together two or more expressions, minimising the expr when possible -func OrExpressions(exprs ...Expr) Expr { +// joinExpressions join together a list of Expr using the baseType as operator (either AndExpr or OrExpr). +func joinExpressions(baseType Expr, exprs ...Expr) Expr { switch len(exprs) { case 0: return nil @@ -383,6 +351,9 @@ func OrExpressions(exprs ...Expr) Expr { default: result := (Expr)(nil) for i, expr := range exprs { + if expr == nil { + continue + } if result == nil { result = expr } else { @@ -394,12 +365,28 @@ func OrExpressions(exprs ...Expr) Expr { } } if !found { - result = &OrExpr{Left: result, Right: expr} + switch baseType.(type) { + case *AndExpr: + result = &AndExpr{Left: result, Right: expr} + case *OrExpr: + result = &OrExpr{Left: result, Right: expr} + } } } } return result } + +} + +// AndExpressions ands together two or more expressions, minimising the expr when possible +func AndExpressions(exprs ...Expr) Expr { + return joinExpressions(&AndExpr{}, exprs...) +} + +// OrExpressions ors together two or more expressions, minimising the expr when possible +func OrExpressions(exprs ...Expr) Expr { + return joinExpressions(&OrExpr{}, exprs...) } // TableFromStatement returns the qualified table name for the query. diff --git a/go/vt/vtgate/planbuilder/abstract/operator.go b/go/vt/vtgate/planbuilder/abstract/operator.go index 31b2618e7f4..0b5c61267e1 100644 --- a/go/vt/vtgate/planbuilder/abstract/operator.go +++ b/go/vt/vtgate/planbuilder/abstract/operator.go @@ -210,13 +210,13 @@ 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, - Replace: sq.ExprsNeedReplace, - ReplaceBy: sq.ReplaceBy, + SelectStatement: subquerySelectStatement, + Inner: opInner, + Type: sq.OpCode, + ArgName: sq.ArgName, + HasValues: sq.HasValues, + ExprsNeedReplace: sq.ExprsNeedReplace, + ReplaceBy: sq.ReplaceBy, }) } } diff --git a/go/vt/vtgate/planbuilder/abstract/subquery.go b/go/vt/vtgate/planbuilder/abstract/subquery.go index ea1b6c905ab..3a98d969f37 100644 --- a/go/vt/vtgate/planbuilder/abstract/subquery.go +++ b/go/vt/vtgate/planbuilder/abstract/subquery.go @@ -34,13 +34,13 @@ var _ Operator = (*SubQuery)(nil) // SubQueryInner stores the subquery information for a select statement type SubQueryInner struct { - Inner Operator - Type engine.PulloutOpcode - SelectStatement *sqlparser.Select - ArgName string - HasValues string - Replace []sqlparser.Expr - ReplaceBy sqlparser.Expr + Inner Operator + Type engine.PulloutOpcode + SelectStatement *sqlparser.Select + ArgName string + HasValues string + ExprsNeedReplace []sqlparser.Expr + ReplaceBy sqlparser.Expr } // TableID implements the Operator interface diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 541e212f94a..9913b15d825 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -178,11 +178,11 @@ 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, - sqToReplaceArg: map[string]*sqlparser.Select{}, - sqToReplaceExpr: map[sqlparser.Expr]sqlparser.Expr{}, + reservedVars: reservedVars, + semTable: semTable, + vschema: vschema, + argToReplaceBySelect: map[string]*sqlparser.Select{}, + exprToReplaceBySqExpr: map[sqlparser.Expr]sqlparser.Expr{}, } return ctx } @@ -217,7 +217,7 @@ func planHorizon(ctx *planningContext, plan logicalPlan, in sqlparser.SelectStat sel: node, } - replaceSubQuery(ctx.sqToReplaceExpr, node) + replaceSubQuery(ctx.exprToReplaceBySqExpr, 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 eaa920cc78b..3202319a46f 100644 --- a/go/vt/vtgate/planbuilder/querytree_transformers.go +++ b/go/vt/vtgate/planbuilder/querytree_transformers.go @@ -324,7 +324,7 @@ func transformRoutePlan(ctx *planningContext, n *routeTree) (*route, error) { Comments: ctx.semTable.Comments, } - replaceSubQuery(ctx.sqToReplaceExpr, sel) + replaceSubQuery(ctx.exprToReplaceBySqExpr, sel) // TODO clean up when gen4 is the only planner var condition sqlparser.Expr @@ -439,8 +439,8 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { } type subQReplacer struct { - sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr - replaced bool + exprToReplaceBySqExpr map[sqlparser.Expr]sqlparser.Expr + replaced bool } func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { @@ -464,7 +464,7 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { var remainder sqlparser.Expr for _, expr := range exprs { found := false - for sqExprToReplace, replaceByExpr := range sqr.sqToReplaceExpr { + for sqExprToReplace, replaceByExpr := range sqr.exprToReplaceBySqExpr { if sqlparser.EqualsExpr(expr, sqExprToReplace) { allReplaceByExprs := sqlparser.SplitAndExpression(nil, replaceBy) allReplaceByExprs = append(allReplaceByExprs, replaceByExpr) @@ -486,9 +486,9 @@ func (sqr *subQReplacer) replacer(cursor *sqlparser.Cursor) bool { return false } -func replaceSubQuery(sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { - if len(sqToReplaceExpr) > 0 { - sqr := &subQReplacer{sqToReplaceExpr: sqToReplaceExpr} +func replaceSubQuery(exprToReplaceBySqExpr map[sqlparser.Expr]sqlparser.Expr, sel *sqlparser.Select) { + if len(exprToReplaceBySqExpr) > 0 { + sqr := &subQReplacer{exprToReplaceBySqExpr: exprToReplaceBySqExpr} 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 d04ee6cb447..a07187fcaab 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -98,15 +98,8 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co if node.Operator != sqlparser.InOp && node.Operator != sqlparser.NotInOp { return } - 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 - } else { + subq, exp := filterSubqueryAndCompareExpr(node) + if subq == nil || exp == nil { return } @@ -139,10 +132,26 @@ func rewriteInSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Co 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) } +func filterSubqueryAndCompareExpr(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) { semTableSQ, found := r.semTable.SubqueryRef[node] if !found || semTableSQ.OpCode != engine.PulloutValue { diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 51c8e4d91a1..18ea9a13e78 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -38,13 +38,14 @@ type planningContext struct { reservedVars *sqlparser.ReservedVars semTable *semantics.SemTable vschema ContextVSchema - // these helps in replacing the argNames with the subquery - sqToReplaceArg map[string]*sqlparser.Select - sqToReplaceExpr map[sqlparser.Expr]sqlparser.Expr + // 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 } func (c planningContext) isSubQueryToReplace(name string) bool { - _, found := c.sqToReplaceArg[name] + _, found := c.argToReplaceBySelect[name] return found } @@ -278,9 +279,9 @@ func rewriteSubqueryDependenciesForJoin(ctx *planningContext, otherTree queryTre } func mergeSubQuery(ctx *planningContext, outer *routeTree, inner *routeTree, subq *abstract.SubQueryInner) (*routeTree, error) { - ctx.sqToReplaceArg[subq.ArgName] = subq.SelectStatement - for _, expr := range subq.Replace { - ctx.sqToReplaceExpr[expr] = subq.ReplaceBy + ctx.argToReplaceBySelect[subq.ArgName] = subq.SelectStatement + for _, expr := range subq.ExprsNeedReplace { + ctx.exprToReplaceBySqExpr[expr] = subq.ReplaceBy } // 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/planbuilder/routetree.go b/go/vt/vtgate/planbuilder/routetree.go index 64dc143cf1c..29bcb4d1fac 100644 --- a/go/vt/vtgate/planbuilder/routetree.go +++ b/go/vt/vtgate/planbuilder/routetree.go @@ -409,7 +409,7 @@ func (rp *routeTree) planIsExpr(ctx *planningContext, node *sqlparser.IsExpr) (b } // makePlanValue transforms the given sqlparser.Expr into a sqltypes.PlanValue. -// If the given sqlparser.Expr is an argument and can be found in the rp.sqToReplaceArg then the +// If the given sqlparser.Expr is an argument and can be found in the rp.argToReplaceBySelect then the // method will stops and return nil values. // 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. diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 8421b66e982..3f48c30a789 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -104,10 +104,13 @@ type ( } subquery struct { - ArgName string - HasValues string - SubQuery *sqlparser.Subquery - OpCode engine.PulloutOpcode + 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 } From f49d2d193b2ed7df4f33572e47c2cd218f7badfe Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Wed, 29 Sep 2021 07:50:27 +0200 Subject: [PATCH 08/10] Addition of comments for SubQueryInner fields Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/abstract/subquery.go | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/subquery.go b/go/vt/vtgate/planbuilder/abstract/subquery.go index 3a98d969f37..7ac4776f163 100644 --- a/go/vt/vtgate/planbuilder/abstract/subquery.go +++ b/go/vt/vtgate/planbuilder/abstract/subquery.go @@ -34,11 +34,33 @@ var _ Operator = (*SubQuery)(nil) // SubQueryInner stores the subquery information for a select statement type SubQueryInner struct { - Inner Operator - Type engine.PulloutOpcode - SelectStatement *sqlparser.Select - ArgName string - HasValues string + // Inner is the Operator inside the parenthesis of the subquery. + // i.e: select (select 1 union select 1), the Inner here would be + // 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 } From efc1cc0d8c1386b70bd4da7a24ac08e214e901f7 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Thu, 30 Sep 2021 07:33:03 +0200 Subject: [PATCH 09/10] Updated plan tests with proper sq_has_values output Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index 2b40daabee8..150d3bd3b19 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt @@ -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 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`" } ] From 50ddb5bde85e37a0eb9a21f5ad55ed37b07bedb6 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Thu, 30 Sep 2021 07:58:34 +0200 Subject: [PATCH 10/10] Order by results in E2E subquery tests to ensure consistent results Signed-off-by: Florent Poinsard --- go/test/endtoend/vtgate/misc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index d7f100996ec..d4cfe57dbeb 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -763,7 +763,7 @@ func TestSubqueriesHasValues(t *testing.T) { defer exec(t, conn, `delete from t1`) exec(t, conn, "insert into t1(id1, id2) values (0,1),(1,2),(2,3),(3,4),(4,5),(5,6)") assertMatches(t, conn, `SELECT id2 FROM t1 WHERE id1 IN (SELECT id1 FROM t1 WHERE id1 > 10)`, `[]`) - assertMatches(t, conn, `SELECT id2 FROM t1 WHERE id1 NOT IN (SELECT id1 FROM t1 WHERE id1 > 10)`, `[[INT64(1)] [INT64(5)] [INT64(2)] [INT64(3)] [INT64(4)] [INT64(6)]]`) + assertMatches(t, conn, `SELECT id2 FROM t1 WHERE id1 NOT IN (SELECT id1 FROM t1 WHERE id1 > 10) ORDER BY id2`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)] [INT64(5)] [INT64(6)]]`) } func TestSelectEqualUniqueOuterJoinRightPredicate(t *testing.T) {