From a553ec286e56243f23f11c3c46b5c287b56a4494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Taylor?= Date: Thu, 25 Jul 2024 12:49:52 +0200 Subject: [PATCH 1/4] bugfix: don't treat join predicates as filter predicates Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/misc/misc_test.go | 14 +++++ .../planbuilder/operators/route_planning.go | 35 +++++------- .../planbuilder/testdata/from_cases.json | 53 +++++++++++++++++++ 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index e768524c2c9..42027fe17a3 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -291,3 +291,17 @@ func TestAnalyze(t *testing.T) { }) } } + +// TestAliasesInOuterJoinQueries tests that aliases work in queries that have outer join clauses. +func TestAliasesInOuterJoinQueries(t *testing.T) { + mcmp, closer := start(t) + defer closer() + + // Insert data into the 2 tables + mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)") + mcmp.Exec("insert into tbl(id, unq_col, nonunq_col) values (1,2,3), (2,5,3), (3, 42, 2)") + + if utils.BinaryIsAtVersion(18, "vtgate") { + mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id") + } +} diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go index 9a4940173e4..93a00154dc1 100644 --- a/go/vt/vtgate/planbuilder/operators/route_planning.go +++ b/go/vt/vtgate/planbuilder/operators/route_planning.go @@ -383,19 +383,23 @@ func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs ops.Operator, joinPr } join := NewApplyJoin(Clone(rhs), Clone(lhs), nil, !inner) - newOp, err := pushJoinPredicates(ctx, joinPredicates, join) - if err != nil { - return nil, nil, err + for _, pred := range joinPredicates { + err := join.AddJoinPredicate(ctx, pred) + if err != nil { + return nil, nil, err + } } - return newOp, rewrite.NewTree("logical join to applyJoin, switching side because LIMIT", newOp), nil + return join, rewrite.NewTree("logical join to applyJoin, switching side because LIMIT", join), nil } join := NewApplyJoin(Clone(lhs), Clone(rhs), nil, !inner) - newOp, err := pushJoinPredicates(ctx, joinPredicates, join) - if err != nil { - return nil, nil, err + for _, pred := range joinPredicates { + err := join.AddJoinPredicate(ctx, pred) + if err != nil { + return nil, nil, err + } } - return newOp, rewrite.NewTree("logical join to applyJoin ", newOp), nil + return join, rewrite.NewTree("logical join to applyJoin ", join), nil } func operatorsToRoutes(a, b ops.Operator) (*Route, *Route) { @@ -614,18 +618,3 @@ func hexEqual(a, b *sqlparser.Literal) bool { } return false } - -func pushJoinPredicates(ctx *plancontext.PlanningContext, exprs []sqlparser.Expr, op *ApplyJoin) (ops.Operator, error) { - if len(exprs) == 0 { - return op, nil - } - - for _, expr := range exprs { - _, err := AddPredicate(ctx, op, expr, true, newFilter) - if err != nil { - return nil, err - } - } - - return op, nil -} diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 6b8e8eba70a..4febb58075b 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -808,6 +808,59 @@ ] } }, + { + "comment": "Outer join with join predicates that only depend on the inner side", + "query": "select 1 from user left join user_extra on user.foo = 42 and user.bar = user_extra.bar", + "plan": { + "QueryType": "SELECT", + "Original": "select 1 from user left join user_extra on user.foo = 42 and user.bar = user_extra.bar", + "Instructions": { + "OperatorType": "Projection", + "Expressions": [ + "1 as 1" + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinVars": { + "user_bar": 1, + "user_foo": 0 + }, + "TableName": "`user`_user_extra", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.foo, `user`.bar from `user` where 1 != 1", + "Query": "select `user`.foo, `user`.bar from `user`", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from user_extra where 1 != 1", + "Query": "select 1 from user_extra where user_extra.bar = :user_bar and :user_foo = 42", + "Table": "user_extra" + } + ] + } + ] + }, + "TablesUsed": [ + "user.user", + "user.user_extra" + ] + } + }, { "comment": "Parenthesized, single chunk", "query": "select user.col from user join (unsharded as m1 join unsharded as m2)", From 113cae78b6ea7edab4ee6ec3037e9541d80e3ffb Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 25 Jul 2024 15:56:17 +0200 Subject: [PATCH 2/4] Empty commit to trigger CI Signed-off-by: Andres Taylor From ee660927d2501a43e709fc9b38b2e81d4f95f07c Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 25 Jul 2024 15:58:40 +0200 Subject: [PATCH 3/4] update test Signed-off-by: Andres Taylor --- go/vt/vtgate/planbuilder/testdata/from_cases.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 4febb58075b..903c59900b4 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -817,7 +817,7 @@ "Instructions": { "OperatorType": "Projection", "Expressions": [ - "1 as 1" + "INT64(1) as 1" ], "Inputs": [ { From 8fbd294f380107089f8f565af2d0ae48501e2229 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Tue, 30 Jul 2024 07:31:07 +0200 Subject: [PATCH 4/4] test: only run on 18 Signed-off-by: Andres Taylor --- go/test/endtoend/vtgate/queries/misc/misc_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 42027fe17a3..123f3c2c97c 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -294,14 +294,17 @@ func TestAnalyze(t *testing.T) { // TestAliasesInOuterJoinQueries tests that aliases work in queries that have outer join clauses. func TestAliasesInOuterJoinQueries(t *testing.T) { + version, err := cluster.GetMajorVersion("vtgate") + require.NoError(t, err) + if version != 18 { + t.Skip("only run on version 18") + } + mcmp, closer := start(t) defer closer() // Insert data into the 2 tables mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)") mcmp.Exec("insert into tbl(id, unq_col, nonunq_col) values (1,2,3), (2,5,3), (3, 42, 2)") - - if utils.BinaryIsAtVersion(18, "vtgate") { - mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id") - } + mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id") }