From 4cb3d1b94ab993dd8f39d0f918c94c0881cc38b1 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 16 May 2024 16:43:48 +0530 Subject: [PATCH 1/5] feat: fix dual merging Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_funcs.go | 20 +++ .../planbuilder/operators/join_merging.go | 19 +-- .../planbuilder/testdata/select_cases.json | 115 ++++++++++++++++++ 3 files changed, 145 insertions(+), 9 deletions(-) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index df201676fae..5e9fd084ad4 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1938,6 +1938,26 @@ func (joinType JoinType) IsCommutative() bool { } } +// IsLeft returns whether the join type is left join type or not. +func (joinType JoinType) IsLeft() bool { + switch joinType { + case LeftJoinType, NaturalLeftJoinType: + return true + default: + return false + } +} + +// IsRight returns whether the join type is right join type or not. +func (joinType JoinType) IsRight() bool { + switch joinType { + case RightJoinType, NaturalRightJoinType: + return true + default: + return false + } +} + // IsInner returns whether the join type is an inner join or not. func (joinType JoinType) IsInner() bool { switch joinType { diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index 5edc812b1b7..536a5303dc4 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -27,17 +27,24 @@ import ( // mergeJoinInputs checks whether two operators can be merged into a single one. // If they can be merged, a new operator with the merged routing is returned // If they cannot be merged, nil is returned. -func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPredicates []sqlparser.Expr, m merger) *Route { +func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPredicates []sqlparser.Expr, m *joinMerger) *Route { lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(lhs, rhs) if lhsRoute == nil { return nil } switch { - // if either side is a dual query, we can always merge them together case a == dual: + // If a dual is on the left side and it is a left join, then we can only merge if the right side is a single sharded routing. + if m.joinType.IsLeft() && !routingB.OpCode().IsSingleShard() { + return nil + } return m.merge(ctx, lhsRoute, rhsRoute, routingB) case b == dual: + // If a dual is on the right side and it is a right join, then we can only merge if the left side is a single sharded routing. + if m.joinType.IsRight() && !routingA.OpCode().IsSingleShard() { + return nil + } return m.merge(ctx, lhsRoute, rhsRoute, routingA) // an unsharded/reference route can be merged with anything going to that keyspace @@ -74,12 +81,6 @@ func prepareInputRoutes(lhs Operator, rhs Operator) (*Route, *Route, Routing, Ro lhsRoute, rhsRoute, routingA, routingB, sameKeyspace := getRoutesOrAlternates(lhsRoute, rhsRoute) a, b := getRoutingType(routingA), getRoutingType(routingB) - if getTypeName(routingA) < getTypeName(routingB) { - // while deciding if two routes can be merged, the LHS/RHS order of the routes is not important. - // for the actual merging, we still need to remember which side was inner and which was outer for subqueries - a, b = b, a - routingA, routingB = routingB, routingA - } return lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace } @@ -178,7 +179,7 @@ func getRoutingType(r Routing) routingType { panic(fmt.Sprintf("switch should be exhaustive, got %T", r)) } -func newJoinMerge(predicates []sqlparser.Expr, joinType sqlparser.JoinType) merger { +func newJoinMerge(predicates []sqlparser.Expr, joinType sqlparser.JoinType) *joinMerger { return &joinMerger{ predicates: predicates, joinType: joinType, diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index d19e07be662..8649492865a 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2043,6 +2043,121 @@ ] } }, + { + "comment": "left join with a dual table on left", + "query": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id=1", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id=1", + "Instructions": { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "L:0,R:0", + "TableName": "dual_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Reference", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select t.title from (select 'hello' as title from dual where 1 != 1) as t where 1 != 1", + "Query": "select t.title from (select 'hello' as title from dual) as t", + "Table": "dual" + }, + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.col from `user` where 1 != 1", + "Query": "select `user`.col from `user` where `user`.id = 1", + "Table": "`user`", + "Values": [ + "1" + ], + "Vindex": "user_index" + } + ] + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, + { + "comment": "right join with a dual table on left", + "query": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id=1", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id=1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual where 1 != 1) as t on `user`.id = 1 where 1 != 1", + "Query": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual) as t on `user`.id = 1", + "Table": "`user`, dual" + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, + { + "comment": "right join with a dual table on right", + "query": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id=1", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id=1", + "Instructions": { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "L:0,R:0", + "TableName": "dual_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Reference", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select t.title from (select 'hello' as title from dual where 1 != 1) as t where 1 != 1", + "Query": "select t.title from (select 'hello' as title from dual) as t", + "Table": "dual" + }, + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.col from `user` where 1 != 1", + "Query": "select `user`.col from `user` where `user`.id = 1", + "Table": "`user`", + "Values": [ + "1" + ], + "Vindex": "user_index" + } + ] + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, { "comment": "Union after into outfile is incorrect", "query": "select id from user into outfile 'out_file_name' union all select id from music", From e912c3cb556a98f70cf425b85af8d46882dc7538 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 22 May 2024 11:00:03 +0530 Subject: [PATCH 2/5] refactor: make code simpler Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_funcs.go | 20 ------------------- .../planbuilder/operators/join_merging.go | 9 +++------ 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 5e9fd084ad4..df201676fae 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1938,26 +1938,6 @@ func (joinType JoinType) IsCommutative() bool { } } -// IsLeft returns whether the join type is left join type or not. -func (joinType JoinType) IsLeft() bool { - switch joinType { - case LeftJoinType, NaturalLeftJoinType: - return true - default: - return false - } -} - -// IsRight returns whether the join type is right join type or not. -func (joinType JoinType) IsRight() bool { - switch joinType { - case RightJoinType, NaturalRightJoinType: - return true - default: - return false - } -} - // IsInner returns whether the join type is an inner join or not. func (joinType JoinType) IsInner() bool { switch joinType { diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index 536a5303dc4..fe357242936 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -35,16 +35,13 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr switch { case a == dual: - // If a dual is on the left side and it is a left join, then we can only merge if the right side is a single sharded routing. - if m.joinType.IsLeft() && !routingB.OpCode().IsSingleShard() { + // If a dual is on the left side and it is a left join (all right joins are changed to left joins), then we can only merge if the right side is a single sharded routing. + if !m.joinType.IsInner() && !routingB.OpCode().IsSingleShard() { return nil } return m.merge(ctx, lhsRoute, rhsRoute, routingB) case b == dual: - // If a dual is on the right side and it is a right join, then we can only merge if the left side is a single sharded routing. - if m.joinType.IsRight() && !routingA.OpCode().IsSingleShard() { - return nil - } + // If a dual is on the right side. return m.merge(ctx, lhsRoute, rhsRoute, routingA) // an unsharded/reference route can be merged with anything going to that keyspace From 8d67079aeb00419ddd27e7096133aae08ee21121 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 28 May 2024 09:17:37 +0530 Subject: [PATCH 3/5] feat: add logic to better merge dual queries Signed-off-by: Manan Gupta --- .../planbuilder/operators/join_merging.go | 41 +++++++++- .../planbuilder/testdata/select_cases.json | 78 ++++++++++++++----- 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index fe357242936..1b6a4dd72f0 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -36,7 +36,7 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr switch { case a == dual: // If a dual is on the left side and it is a left join (all right joins are changed to left joins), then we can only merge if the right side is a single sharded routing. - if !m.joinType.IsInner() && !routingB.OpCode().IsSingleShard() { + if !m.joinType.IsInner() && !willBecomeSingleShardOnFilter(ctx, rhsRoute, joinPredicates) { return nil } return m.merge(ctx, lhsRoute, rhsRoute, routingB) @@ -69,6 +69,45 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr } } +// willBecomeSingleShardOnFilter returns whether the given route will become a single sharded route after pushing the join predicates. +func willBecomeSingleShardOnFilter(ctx *plancontext.PlanningContext, a *Route, joinPredicates []sqlparser.Expr) bool { + // If the routing is already single sharded, it will remain so even after we push more predicates. + if a.Routing.OpCode().IsSingleShard() { + return true + } + + // Go over all the join predicates. + for _, predicate := range joinPredicates { + comparison, ok := predicate.(*sqlparser.ComparisonExpr) + if !ok { + continue + } + if comparison.Operator != sqlparser.EqualOp { + continue + } + left := comparison.Left + right := comparison.Right + + lVindex := findColumnVindex(ctx, a, left) + if lVindex == nil { + left, right = right, left + lVindex = findColumnVindex(ctx, a, left) + } + + if lVindex == nil || !lVindex.IsUnique() { + continue + } + // If the predicate is an equal comparison with the left side having a unique column vindex, + // and the right side is a literal, then we know + // pushing this predicate will make the route a single sharded route. + if sqlparser.IsLiteral(right) { + return true + } + } + + return false +} + func prepareInputRoutes(lhs Operator, rhs Operator) (*Route, *Route, Routing, Routing, routingType, routingType, bool) { lhsRoute, rhsRoute := operatorsToRoutes(lhs, rhs) if lhsRoute == nil || rhsRoute == nil { diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index 8649492865a..f9488f9c206 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2044,11 +2044,34 @@ } }, { - "comment": "left join with a dual table on left", + "comment": "left join with a dual table on left - merge-able", "query": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id=1", "plan": { "QueryType": "SELECT", "Original": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id=1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select t.title, `user`.col from (select 'hello' as title from dual where 1 != 1) as t left join `user` on `user`.id = 1 where 1 != 1", + "Query": "select t.title, `user`.col from (select 'hello' as title from dual) as t left join `user` on `user`.id = 1", + "Table": "`user`, dual" + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, + { + "comment": "left join with a dual table on left - non-merge-able", + "query": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id <= 4", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id <= 4", "Instructions": { "OperatorType": "Join", "Variant": "LeftJoin", @@ -2068,18 +2091,14 @@ }, { "OperatorType": "Route", - "Variant": "EqualUnique", + "Variant": "Scatter", "Keyspace": { "Name": "user", "Sharded": true }, "FieldQuery": "select `user`.col from `user` where 1 != 1", - "Query": "select `user`.col from `user` where `user`.id = 1", - "Table": "`user`", - "Values": [ - "1" - ], - "Vindex": "user_index" + "Query": "select `user`.col from `user` where `user`.id <= 4", + "Table": "`user`" } ] }, @@ -2091,10 +2110,10 @@ }, { "comment": "right join with a dual table on left", - "query": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id=1", + "query": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id<=4", "plan": { "QueryType": "SELECT", - "Original": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id=1", + "Original": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id<=4", "Instructions": { "OperatorType": "Route", "Variant": "Scatter", @@ -2102,8 +2121,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual where 1 != 1) as t on `user`.id = 1 where 1 != 1", - "Query": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual) as t on `user`.id = 1", + "FieldQuery": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual where 1 != 1) as t on `user`.id <= 4 where 1 != 1", + "Query": "select t.title, `user`.col from `user` left join (select 'hello' as title from dual) as t on `user`.id <= 4", "Table": "`user`, dual" }, "TablesUsed": [ @@ -2113,11 +2132,34 @@ } }, { - "comment": "right join with a dual table on right", + "comment": "right join with a dual table on right - merge-able", "query": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id=1", "plan": { "QueryType": "SELECT", "Original": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id=1", + "Instructions": { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select t.title, `user`.col from (select 'hello' as title from dual where 1 != 1) as t left join `user` on `user`.id = 1 where 1 != 1", + "Query": "select t.title, `user`.col from (select 'hello' as title from dual) as t left join `user` on `user`.id = 1", + "Table": "`user`, dual" + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, + { + "comment": "right join with a dual table on right - non-merge-able", + "query": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id>=4", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id>=4", "Instructions": { "OperatorType": "Join", "Variant": "LeftJoin", @@ -2137,18 +2179,14 @@ }, { "OperatorType": "Route", - "Variant": "EqualUnique", + "Variant": "Scatter", "Keyspace": { "Name": "user", "Sharded": true }, "FieldQuery": "select `user`.col from `user` where 1 != 1", - "Query": "select `user`.col from `user` where `user`.id = 1", - "Table": "`user`", - "Values": [ - "1" - ], - "Vindex": "user_index" + "Query": "select `user`.col from `user` where `user`.id >= 4", + "Table": "`user`" } ] }, From 8c00bbe7427f8f0f20db0a0c23d7fc63012bf388 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 12 Jun 2024 10:57:57 +0530 Subject: [PATCH 4/5] feat: use clone and push join predicates on rhs before merging with dual query Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/gen4/gen4_test.go | 20 ++++++++ .../planbuilder/operators/join_merging.go | 50 ++++--------------- .../planbuilder/testdata/select_cases.json | 16 ++++-- 3 files changed, 41 insertions(+), 45 deletions(-) diff --git a/go/test/endtoend/vtgate/gen4/gen4_test.go b/go/test/endtoend/vtgate/gen4/gen4_test.go index f284f85e883..bb1b06157f3 100644 --- a/go/test/endtoend/vtgate/gen4/gen4_test.go +++ b/go/test/endtoend/vtgate/gen4/gen4_test.go @@ -489,3 +489,23 @@ func TestPercentageAndUnderscore(t *testing.T) { mcmp.Exec(`select a.tcol1 from t2 a join t2 b where a.tcol1 = b.tcol2 group by a.tcol1 having repeat(a.tcol1,min(a.id)) = "C_DC_D" order by a.tcol1`) mcmp.Exec(`select a.tcol1 from t2 a join t2 b where a.tcol1 = b.tcol2 group by a.tcol1 having repeat(a.tcol1,min(a.id)) = "C\_DC\_D" order by a.tcol1`) } + +// TestDualJoinQueries tests that queries having a join between a dual query and another query work as intended. +func TestDualJoinQueries(t *testing.T) { + mcmp, closer := start(t) + defer closer() + + mcmp.Exec(`insert into t2(id, tcol1, tcol2) values (1, 'ABC', 'ABC'),(2, 'DEF', 'DEF')`) + + // Left join with a dual table on left - merge-able + mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) + mcmp.AssertMatches("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1", `[[VARCHAR("DEF") NULL]]`) + + // Left join with a dual table on left - non-merge-able + mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) + mcmp.AssertMatches("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1", `[[VARCHAR("DEF") NULL]]`) + + // right join with a dual table on left + mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t right join t2 t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) + +} diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index 1b6a4dd72f0..8bba8ba57d9 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -35,11 +35,18 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr switch { case a == dual: + // We clone the right hand side and try and push all the join predicates that are solved entirely by that side. // If a dual is on the left side and it is a left join (all right joins are changed to left joins), then we can only merge if the right side is a single sharded routing. - if !m.joinType.IsInner() && !willBecomeSingleShardOnFilter(ctx, rhsRoute, joinPredicates) { + rhsClone := Clone(rhs).(*Route) + for _, predicate := range joinPredicates { + if ctx.SemTable.DirectDeps(predicate).IsSolvedBy(TableID(rhsClone)) { + rhsClone.AddPredicate(ctx, predicate) + } + } + if !m.joinType.IsInner() && !rhsClone.Routing.OpCode().IsSingleShard() { return nil } - return m.merge(ctx, lhsRoute, rhsRoute, routingB) + return m.merge(ctx, lhsRoute, rhsClone, rhsClone.Routing) case b == dual: // If a dual is on the right side. return m.merge(ctx, lhsRoute, rhsRoute, routingA) @@ -69,45 +76,6 @@ func mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPr } } -// willBecomeSingleShardOnFilter returns whether the given route will become a single sharded route after pushing the join predicates. -func willBecomeSingleShardOnFilter(ctx *plancontext.PlanningContext, a *Route, joinPredicates []sqlparser.Expr) bool { - // If the routing is already single sharded, it will remain so even after we push more predicates. - if a.Routing.OpCode().IsSingleShard() { - return true - } - - // Go over all the join predicates. - for _, predicate := range joinPredicates { - comparison, ok := predicate.(*sqlparser.ComparisonExpr) - if !ok { - continue - } - if comparison.Operator != sqlparser.EqualOp { - continue - } - left := comparison.Left - right := comparison.Right - - lVindex := findColumnVindex(ctx, a, left) - if lVindex == nil { - left, right = right, left - lVindex = findColumnVindex(ctx, a, left) - } - - if lVindex == nil || !lVindex.IsUnique() { - continue - } - // If the predicate is an equal comparison with the left side having a unique column vindex, - // and the right side is a literal, then we know - // pushing this predicate will make the route a single sharded route. - if sqlparser.IsLiteral(right) { - return true - } - } - - return false -} - func prepareInputRoutes(lhs Operator, rhs Operator) (*Route, *Route, Routing, Routing, routingType, routingType, bool) { lhsRoute, rhsRoute := operatorsToRoutes(lhs, rhs) if lhsRoute == nil || rhsRoute == nil { diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index 253deab6e45..ffa1b26f4bc 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2080,14 +2080,18 @@ "Original": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id=1", "Instructions": { "OperatorType": "Route", - "Variant": "Scatter", + "Variant": "EqualUnique", "Keyspace": { "Name": "user", "Sharded": true }, "FieldQuery": "select t.title, `user`.col from (select 'hello' as title from dual where 1 != 1) as t left join `user` on `user`.id = 1 where 1 != 1", "Query": "select t.title, `user`.col from (select 'hello' as title from dual) as t left join `user` on `user`.id = 1", - "Table": "`user`, dual" + "Table": "`user`, dual", + "Values": [ + "1" + ], + "Vindex": "user_index" }, "TablesUsed": [ "main.dual", @@ -2168,14 +2172,18 @@ "Original": "select t.title, user.col from user right join (select 'hello' as title) as t on user.id=1", "Instructions": { "OperatorType": "Route", - "Variant": "Scatter", + "Variant": "EqualUnique", "Keyspace": { "Name": "user", "Sharded": true }, "FieldQuery": "select t.title, `user`.col from (select 'hello' as title from dual where 1 != 1) as t left join `user` on `user`.id = 1 where 1 != 1", "Query": "select t.title, `user`.col from (select 'hello' as title from dual) as t left join `user` on `user`.id = 1", - "Table": "`user`, dual" + "Table": "`user`, dual", + "Values": [ + "1" + ], + "Vindex": "user_index" }, "TablesUsed": [ "main.dual", From 84c968f8fbffe009052dae5a2a0d1a5a97e12f59 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 12 Jun 2024 15:10:56 +0530 Subject: [PATCH 5/5] feat: fix join to use direct dependencies, and add more e2e tests Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/gen4/gen4_test.go | 21 ++++++--- go/vt/vtgate/planbuilder/operators/joins.go | 2 +- .../planbuilder/testdata/select_cases.json | 45 +++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/go/test/endtoend/vtgate/gen4/gen4_test.go b/go/test/endtoend/vtgate/gen4/gen4_test.go index bb1b06157f3..a242ef0bb7a 100644 --- a/go/test/endtoend/vtgate/gen4/gen4_test.go +++ b/go/test/endtoend/vtgate/gen4/gen4_test.go @@ -498,14 +498,25 @@ func TestDualJoinQueries(t *testing.T) { mcmp.Exec(`insert into t2(id, tcol1, tcol2) values (1, 'ABC', 'ABC'),(2, 'DEF', 'DEF')`) // Left join with a dual table on left - merge-able - mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) - mcmp.AssertMatches("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1", `[[VARCHAR("DEF") NULL]]`) + mcmp.Exec("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1") + mcmp.Exec("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id=1 and t.title = t2.tcol1") // Left join with a dual table on left - non-merge-able - mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) - mcmp.AssertMatches("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1", `[[VARCHAR("DEF") NULL]]`) + mcmp.Exec("select t.title, t2.id from (select 'ABC' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1") + mcmp.Exec("select t.title, t2.id from (select 'DEF' as title) as t left join t2 on t2.id < 2 and t.title = t2.tcol1") // right join with a dual table on left - mcmp.AssertMatches("select t.title, t2.id from (select 'ABC' as title) as t right join t2 t.title = t2.tcol1", `[[VARCHAR("ABC") INT64(1)]]`) + mcmp.Exec("select t.title, t2.id from (select 'ABC' as title) as t right join t2 on t.title = t2.tcol1") + + // Right join with a dual table on right - merge-able + mcmp.Exec("select t.title, t2.id from t2 right join (select 'ABC' as title) as t on t2.id=1 and t.title = t2.tcol1") + mcmp.Exec("select t.title, t2.id from t2 right join (select 'DEF' as title) as t on t2.id=1 and t.title = t2.tcol1") + + // Right join with a dual table on right - non-merge-able + mcmp.Exec("select t.title, t2.id from t2 right join (select 'ABC' as title) as t on t2.id < 2 and t.title = t2.tcol1") + mcmp.Exec("select t.title, t2.id from t2 right join (select 'DEF' as title) as t on t2.id < 2 and t.title = t2.tcol1") + + // left join with a dual table on right + mcmp.Exec("select t.title, t2.id from t2 left join (select 'ABC' as title) as t on t.title = t2.tcol1") } diff --git a/go/vt/vtgate/planbuilder/operators/joins.go b/go/vt/vtgate/planbuilder/operators/joins.go index d0d0fa770c8..86cdf06fe7e 100644 --- a/go/vt/vtgate/planbuilder/operators/joins.go +++ b/go/vt/vtgate/planbuilder/operators/joins.go @@ -44,7 +44,7 @@ func AddPredicate( joinPredicates bool, newFilter func(Operator, sqlparser.Expr) Operator, ) Operator { - deps := ctx.SemTable.RecursiveDeps(expr) + deps := ctx.SemTable.DirectDeps(expr) switch { case deps.IsSolvedBy(TableID(join.GetLHS())): // predicates can always safely be pushed down to the lhs if that is all they depend on diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index ffa1b26f4bc..aa022a52859 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2141,6 +2141,51 @@ ] } }, + { + "comment": "left join with dual non-merge-able with predicate with cross dependencies", + "query": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id <= 4 and t.title = user.col", + "plan": { + "QueryType": "SELECT", + "Original": "select t.title, user.col from (select 'hello' as title) as t left join user on user.id <= 4 and t.title = user.col", + "Instructions": { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "L:0,R:0", + "JoinVars": { + "t_title": 0 + }, + "TableName": "dual_`user`", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Reference", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select t.title from (select 'hello' as title from dual where 1 != 1) as t where 1 != 1", + "Query": "select t.title from (select 'hello' as title from dual) as t", + "Table": "dual" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.col from `user` where 1 != 1", + "Query": "select `user`.col from `user` where `user`.col = :t_title and `user`.id <= 4", + "Table": "`user`" + } + ] + }, + "TablesUsed": [ + "main.dual", + "user.user" + ] + } + }, { "comment": "right join with a dual table on left", "query": "select t.title, user.col from (select 'hello' as title) as t right join user on user.id<=4",