From 4c6ea1021b38ae4067364d0c395a723642899d50 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 28 Sep 2021 10:33:35 +0200 Subject: [PATCH 1/2] Keep track of left and right predicate in outer joins Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/abstract/left_join.go | 6 ++++-- go/vt/vtgate/planbuilder/route_planning.go | 1 + go/vt/vtgate/planbuilder/testdata/filter_cases.txt | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/left_join.go b/go/vt/vtgate/planbuilder/abstract/left_join.go index 49cfd251f8c..22f2b5e9617 100644 --- a/go/vt/vtgate/planbuilder/abstract/left_join.go +++ b/go/vt/vtgate/planbuilder/abstract/left_join.go @@ -34,10 +34,12 @@ var _ Operator = (*LeftJoin)(nil) // PushPredicate implements the Operator interface func (lj *LeftJoin) PushPredicate(expr sqlparser.Expr, semTable *semantics.SemTable) error { deps := semTable.RecursiveDeps(expr) - if deps.IsSolvedBy(lj.Left.TableID()) { + switch { + case deps.IsSolvedBy(lj.Left.TableID()): return lj.Left.PushPredicate(expr, semTable) + case deps.IsSolvedBy(lj.Right.TableID()): + return lj.Right.PushPredicate(expr, semTable) } - return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "Cannot push predicate: %s", sqlparser.String(expr)) } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index dc1e85b4f69..e72af3d4ef1 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -1142,6 +1142,7 @@ func createRoutePlanForOuter(ctx *planningContext, aRoute, bRoute *routeTree, ne right: outer, pred: sqlparser.AndExpressions(joinPredicates...), }), + predicates: append(aRoute.predicates, bRoute.predicates...), keyspace: aRoute.keyspace, vindexPreds: append(aRoute.vindexPreds, bRoute.vindexPreds...), } diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt index d216a9c6487..de81d507424 100644 --- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt @@ -670,6 +670,7 @@ Gen4 plan same as above "Vindex": "user_index" } } +Gen4 plan same as above # Multi-route unique vindex constraint "select user_extra.id from user join user_extra on user.col = user_extra.col where user.id = 5" From 5006322dc43fa279fdc9bc2bef10a02b34b0f162 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 28 Sep 2021 12:02:56 +0200 Subject: [PATCH 2/2] Addition of an end to end tests to reproduce the outer join predicate issue Signed-off-by: Florent Poinsard --- go/test/endtoend/vtgate/misc_test.go | 18 ++++++++++++++++++ .../planbuilder/gen4_compare_v3_planner.go | 18 +++++++++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 66db6549c95..386a0b10057 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -753,6 +753,24 @@ 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 TestSelectEqualUniqueOuterJoinRightPredicate(t *testing.T) { + defer cluster.PanicHandler(t) + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + exec(t, conn, `delete from t1`) + exec(t, conn, `delete from t2`) + defer func() { + exec(t, conn, `delete from t1`) + exec(t, conn, `delete from t2`) + }() + exec(t, conn, "insert into t1(id1, id2) values (0,10),(1,9),(2,8),(3,7),(4,6),(5,5)") + exec(t, conn, "insert into t2(id3, id4) values (0,20),(1,19),(2,18),(3,17),(4,16),(5,15)") + assertMatches(t, conn, `SELECT id3 FROM t1 LEFT JOIN t2 ON t1.id1 = t2.id3 WHERE t2.id3 = 10`, `[]`) +} + func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) { t.Helper() qr := exec(t, conn, query) diff --git a/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go b/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go index 072e7ceacb4..59b5bc03276 100644 --- a/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_compare_v3_planner.go @@ -37,15 +37,6 @@ func gen4CompareV3Planner(query string) func(sqlparser.Statement, *sqlparser.Res // plan statement using Gen4 gen4Primitive, gen4Err := planWithPlannerVersion(statement, vars, ctxVSchema, query, Gen4) - // if onlyGen4 is set to true or Gen4's instruction contain a lock primitive, - // we use only Gen4's primitive and exit early without using V3's. - // since lock primitives can imply the creation or deletion of locks, - // we want to execute them once using Gen4 to avoid the duplicated locks - // or double lock-releases. - if onlyGen4 || hasLockPrimitive(gen4Primitive) { - return gen4Primitive, gen4Err - } - // get V3's plan v3Primitive, v3Err := planWithPlannerVersion(statement, vars, ctxVSchema, query, V3) @@ -55,6 +46,15 @@ func gen4CompareV3Planner(query string) func(sqlparser.Statement, *sqlparser.Res return nil, err } + // if onlyGen4 is set to true or Gen4's instruction contain a lock primitive, + // we use only Gen4's primitive and exit early without using V3's. + // since lock primitives can imply the creation or deletion of locks, + // we want to execute them once using Gen4 to avoid the duplicated locks + // or double lock-releases. + if onlyGen4 || hasLockPrimitive(gen4Primitive) { + return gen4Primitive, gen4Err + } + return &engine.Gen4CompareV3{ V3: v3Primitive, Gen4: gen4Primitive,