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

[release-19.0] bugfix: don't treat join predicates as filter predicates (#16472) #16474

Merged
merged 1 commit into from
Jul 31, 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
11 changes: 11 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,17 @@ func TestTransactionModeVar(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)")
mcmp.ExecWithColumnCompare("select * from t1 t left join tbl on t.id1 = 666 and t.id2 = tbl.id")
}

func TestAlterTableWithView(t *testing.T) {
mcmp, closer := start(t)
defer closer()
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/planbuilder/operators/apply_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@
}

firstPart := fmt.Sprintf("on %s columns: %s", fn(aj.JoinPredicates), fn(aj.JoinColumns))
if aj.LeftJoin {
firstPart = "LEFT JOIN " + firstPart

Check warning on line 281 in go/vt/vtgate/planbuilder/operators/apply_join.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/planbuilder/operators/apply_join.go#L280-L281

Added lines #L280 - L281 were not covered by tests
}

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 @@ -358,13 +358,18 @@ func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPredic
}

join := NewApplyJoin(ctx, Clone(rhs), Clone(lhs), nil, !inner)
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, !inner)
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 @@ -583,15 +588,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 @@ -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)",
Expand Down
Loading