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.

Sign up for GitHub

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

planbuilder: use OR for not in comparisons #14607

Merged
merged 3 commits into from
Nov 27, 2023
Merged

planbuilder: use OR for not in comparisons #14607

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56ffeab
plabuilder: use OR for not in comparisons
systay Nov 27, 2023
9c40c9b
test: make sure we have e2e tests for NOT IN situations
systay Nov 27, 2023
6c394ef
test: don't tests run on v18
systay Nov 27, 2023
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
5 changes: 4 additions & 1 deletion go/vt/vtgate/planbuilder/operators/subquery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,10 @@ func (sq *SubQuery) settleFilter(ctx *plancontext.PlanningContext, outer ops.Ope
predicates = append(predicates, sqlparser.NewArgument(hasValuesArg()), rhsPred)
sq.SubqueryValueName = sq.ArgName
case opcode.PulloutNotIn:
predicates = append(predicates, sqlparser.NewNotExpr(sqlparser.NewArgument(hasValuesArg())), rhsPred)
predicates = append(predicates, &sqlparser.OrExpr{
Left: sqlparser.NewNotExpr(sqlparser.NewArgument(hasValuesArg())),
Right: rhsPred,
})
sq.SubqueryValueName = sq.ArgName
case opcode.PulloutValue:
predicates = append(predicates, rhsPred)
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/planbuilder/testdata/filter_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1966,7 +1966,7 @@
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where not :__sq_has_values and id not in ::__sq1",
"Query": "select id from `user` where not :__sq_has_values or id not in ::__sq1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question. It seems that in this scenario, we can remove the "not :__sq_has_values" condition. Why do we need to convert it to "or"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent question!

We have already built the queries by this time, so we need to be able to send down something that is valid SQL.
NOT IN () does not even parse - we need to have at least one value there.
We can't stick NULL there, NOT IN (NULL) evaluates to NULL, not true.
We just put in the fake value 0. Just to make sure that we end up with true for the empty results, even if the column we are comparing against does contain a 0, we add this not :__sq_has_values that will return true no matter what.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is important, we would need to come up with a value that we put into the ::__sq1 tuple that guarantees that id not in ::__sq1 returns true for all values of id. We can make it very unlikely, but not guarantee that we will no clash, right?

"Table": "`user`"
}
]
Expand Down Expand Up @@ -2503,7 +2503,7 @@
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where not :__sq_has_values and id not in ::__sq1 and :__sq_has_values1 and id in ::__vals",
"Query": "select id from `user` where (not :__sq_has_values or id not in ::__sq1) and :__sq_has_values1 and id in ::__vals",
"Table": "`user`",
"Values": [
"::__sq2"
Expand Down Expand Up @@ -2950,7 +2950,7 @@
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where id = 5 and id in (select user_extra.col from user_extra where user_extra.user_id = 5) and not :__sq_has_values and id not in ::__sq1",
"Query": "select id from `user` where id = 5 and id in (select user_extra.col from user_extra where user_extra.user_id = 5) and (not :__sq_has_values or id not in ::__sq1)",
"Table": "`user`",
"Values": [
"5"
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/tpch_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,7 +1594,7 @@
"Sharded": true
},
"FieldQuery": "select ps_suppkey, weight_string(ps_suppkey), ps_partkey from partsupp where 1 != 1",
"Query": "select ps_suppkey, weight_string(ps_suppkey), ps_partkey from partsupp where not :__sq_has_values and ps_suppkey not in ::__sq1",
"Query": "select ps_suppkey, weight_string(ps_suppkey), ps_partkey from partsupp where not :__sq_has_values or ps_suppkey not in ::__sq1",
"Table": "partsupp"
}
]
Expand Down