-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix query planning for complex queries with impossible conditions. #19003
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Arthur Schreiber <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
| } | ||
| }, | ||
| { | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: fix the indentation here.
| "Sharded": true | ||
| }, | ||
| "FieldQuery": "select id from (select music.id from music where 1 != 1) as _inner where 1 != 1", | ||
| "Query": "select id from (select music.id from music where 0) as _inner limit 100" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All ANDed conditions are now correctly replaced by 0, and the final route variant is None.
|
I cherry picked this fix into my fork reproducing our bug in #18987 and confirmed that it does the expected "Inputs": [
{
"InputName": "SubQuery",
"OperatorType": "Limit",
"Count": "100",
"Offset": "0",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "None",
"Keyspace": {
"Name": "foobar_ks",
"Sharded": true
},
"FieldQuery": "select id, subquery_for_limit.updated_at from (select foobar.id, foobar.updated_at from foobar where 1 != 1) as subquery_for_limit where 1 != 1",
"OrderBy": "1 DESC",
"Query": "select id, subquery_for_limit.updated_at from (select foobar.id, foobar.updated_at from foobar where 0) as subquery_for_limit order by subquery_for_limit.updated_at desc limit 100"
}
]
},
] |
Description
This fixes the issue described in #18987, but using a different / more generic approach compared to #19002.
First, this changes
*SemTable.CopySemanticInfoto only copy type information, and only copy dependency information when copying fromsqlparser.Exprtosqlparser.ColName.This also fixes another issue where the correlation logic for subqueries was incorrect and we'd end up marking too many subqueries correlated even though in fact they were not correlated. This was previously hidden by the dependency information copy that happened in
*SemTable.CopySemanticInfo.Related Issue(s)
See #19002 and #18987
Checklist
Deployment Notes
AI Disclosure