-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
plan: support subquery in Do
statement
#8343
Conversation
/run-all-tests |
@@ -34,7 +35,7 @@ func getUsedList(usedCols []*expression.Column, schema *expression.Schema) []boo | |||
for _, col := range usedCols { | |||
idx := schema.ColumnIndex(col) | |||
if idx == -1 { | |||
log.Errorf("Can't find column %s from schema %s.", col, schema) | |||
panic(fmt.Sprintf("Can't find column %s from schema %s.", col, schema)) |
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.
Why panic?
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.
Because if idx == -1
, in line 40, used[idx] = true
would panic also, to make the error message in log clearer, I made this change.
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.
LGTM
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.
LGTM
What problem does this PR solve?
Before this PR:
while in MySQL:
MySQL states in https://dev.mysql.com/doc/refman/5.7/en/do.html that:
The documentation does not explicitly specify the boundary to which
Do
is compatible withSELECT
. Since this case experimentally works in MySQL, we support it in TiDB.What is changed and how it works?
Do
;Projection
specially added forDo
being eliminated in optimizer;Check List
Tests
Side effects
N/A
Related changes
This change is