-
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
executor: fix runtime error when use on uplicate key update #29207
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Welcome @Orion7r! |
/cc @AilinKid @guo-shaoge |
@@ -359,7 +359,7 @@ func (e *InsertExec) initEvalBuffer4Dup() { | |||
evalBufferTypes = append(evalBufferTypes, &col.FieldType) | |||
} | |||
if extraLen > 0 { | |||
evalBufferTypes = append(evalBufferTypes, e.SelectExec.base().retFieldTypes[numWritableCols:]...) | |||
evalBufferTypes = append(evalBufferTypes, e.SelectExec.base().retFieldTypes[0:]...) |
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.
0
is not reasonable here. IMHO, we should usee.rowLen
- We also have some problem in line 371, you can reproduce by using the following case. The second insertStmt doesn't work.
drop table if exists t1;
drop table if exists t2;
create table t1(c1 int, c2 int, c3 varchar(100), unique key(c3));
create table t2(c1 int);
insert into t2 values(1);
insert into t1(c3) select "abc" from t2 on duplicate key update t1.c3 = t2.c1;
insert into t1(c3) select "abc" from t2 on duplicate key update t1.c3 = t2.c1;
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.
yes, you're right, but it shouldn't be e.rowlen
either.
I will think about how to solve this problem.
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.
Excuse me, but do we have any update yet?
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/38361edb4b0e2de9dd0c1af49a8e515e5e1ac6fd |
/run-check_dev_2 |
1 similar comment
/run-check_dev_2 |
Ping, @Orion7r are you going to continue fixing this issue? If you don't have time, we may have to try to fix it by ourselves and close this PR, since this issue is currently blocking. |
sry,I have no time to continue this issue,maybe you can try to fix it. thanks. |
Thanks for your help. |
What problem does this PR solve?
Issue Number: close #28078
What is changed and how it works?
When select is used in combination with on duplicate key update, there will be a runtime error problem, that is, slice bounds out of range, which is simply fixed here.
Release note