Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions go/vt/vtgate/planbuilder/operators/apply_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
25 changes: 9 additions & 16 deletions go/vt/vtgate/planbuilder/operators/route_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
53 changes: 53 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/from_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
Loading