bugfix: don't treat join predicates as filter predicates (#16472)

Signed-off-by: Andres Taylor <andres@planetscale.com>
vitessio · Jul 25, 2024 · 59408f9 · 59408f9
1 parent 88d7033
commit 59408f9
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 18 deletions.
diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go
@@ -371,6 +371,9 @@ func TestAliasesInOuterJoinQueries(t *testing.T) {
 	mcmp.ExecWithColumnCompare("select t1.id1 as t0, t1.id1 as t1, tbl.unq_col as col from t1 left outer join tbl on t1.id2 = tbl.nonunq_col order by t1.id2 limit 2 offset 2")
 	mcmp.ExecWithColumnCompare("select t1.id1 as t0, t1.id1 as t1, count(*) as leCount from t1 left outer join tbl on t1.id2 = tbl.nonunq_col group by 1, t1")
 	mcmp.ExecWithColumnCompare("select t.id1, t.id2, derived.unq_col from t1 t join (select id, unq_col, nonunq_col from tbl) as derived on t.id2 = derived.nonunq_col")
+	if utils.BinaryIsAtLeastAtVersion(21, "vtgate") {
+		mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id")
+	}
 }
 
 func TestAlterTableWithView(t *testing.T) {

diff --git a/go/vt/vtgate/planbuilder/operators/apply_join.go b/go/vt/vtgate/planbuilder/operators/apply_join.go
@@ -37,8 +37,6 @@ type (
 		// JoinType is permitted to store only 3 of the possible values
 		// NormalJoinType, StraightJoinType and LeftJoinType.
 		JoinType sqlparser.JoinType
-		// LeftJoin will be true in the case of an outer join
-		LeftJoin bool
 
 		// JoinColumns keeps track of what AST expression is represented in the Columns array
 		JoinColumns *applyJoinColumns
@@ -344,6 +342,10 @@ func (aj *ApplyJoin) ShortDescription() string {
 	}
 
 	firstPart := fmt.Sprintf("on %s columns: %s", fn(aj.JoinPredicates), fn(aj.JoinColumns))
+	if aj.JoinType == sqlparser.LeftJoinType {
+		firstPart = "LEFT JOIN " + firstPart
+	}
+
 	if len(aj.ExtraLHSVars) == 0 {
 		return firstPart
 	}

diff --git a/go/vt/vtgate/planbuilder/operators/route_planning.go b/go/vt/vtgate/planbuilder/operators/route_planning.go
@@ -305,13 +305,18 @@ func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPredic
 		}
 
 		join := NewApplyJoin(ctx, Clone(rhs), Clone(lhs), nil, joinType)
-		newOp := pushJoinPredicates(ctx, joinPredicates, join)
-		return newOp, Rewrote("logical join to applyJoin, switching side because LIMIT")
+		for _, pred := range joinPredicates {
+			join.AddJoinPredicate(ctx, pred)
+		}
+		return join, Rewrote("logical join to applyJoin, switching side because LIMIT")
 	}
 
 	join := NewApplyJoin(ctx, Clone(lhs), Clone(rhs), nil, joinType)
-	newOp := pushJoinPredicates(ctx, joinPredicates, join)
-	return newOp, Rewrote("logical join to applyJoin ")
+	for _, pred := range joinPredicates {
+		join.AddJoinPredicate(ctx, pred)
+	}
+
+	return join, Rewrote("logical join to applyJoin ")
 }
 
 func operatorsToRoutes(a, b Operator) (*Route, *Route) {
@@ -530,15 +535,3 @@ func hexEqual(a, b *sqlparser.Literal) bool {
 	}
 	return false
 }
-
-func pushJoinPredicates(ctx *plancontext.PlanningContext, exprs []sqlparser.Expr, op *ApplyJoin) Operator {
-	if len(exprs) == 0 {
-		return op
-	}
-
-	for _, expr := range exprs {
-		AddPredicate(ctx, op, expr, true, newFilterSinglePredicate)
-	}
-
-	return op
-}
diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json
@@ -784,6 +784,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)",