-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: fix updating the column value when the column is dropping and in WriteOnly state #15539
Conversation
PTAL @bb7133 @winoros @crazycs520 |
Codecov Report
@@ Coverage Diff @@
## master #15539 +/- ##
===========================================
Coverage 80.5019% 80.5019%
===========================================
Files 502 502
Lines 134644 134644
===========================================
Hits 108391 108391
Misses 17776 17776
Partials 8477 8477 |
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.
Rest 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
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
/run-unit-test |
/run-all-tests |
@zimulala merge failed. |
/merge |
Your auto merge job has been accepted, waiting for 15544 |
/run-all-tests |
@zimulala merge failed. |
/merge |
/run-all-tests |
@zimulala merge failed. |
/merge |
/run-all-tests |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.0 in PR #15576 |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.1 in PR #15577 |
@@ -3226,9 +3226,42 @@ func (b *PlanBuilder) buildUpdate(ctx context.Context, update *ast.UpdateStmt) ( | |||
tblID2table[id], _ = b.is.TableByID(id) | |||
} | |||
updt.TblColPosInfos, err = buildColumns2Handle(updt.OutputNames(), tblID2Handle, tblID2table, true) | |||
if err == nil { | |||
err = checkUpdateList(b.ctx, tblID2table, updt) |
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.
it seems we only check it in updateExec
above normal executor
..so updateExec
above point-get
maybe hit this bug? 😭
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.
@lysu mmm... Then I should fix this or you should fix it?
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.
let me see see- -
I'd fixed in #23491 during a refactor, but maybe it's not suitable to cherry-pick to 4.0 or 3.0
What problem does this PR solve?
Issue Number: close #15386
Do the following:
The result:
Problem Summary:
When the column is dropping and in WriteOnly state, the value of this column cannot be explicitly updated.
What is changed and how it works?
What's Changed:
When the column is in a falling state and in WriteOnly state, if we update this column value explicitly, an error will be returned.
Related changes
Check List
Tests
Side effects
Release note