planner: push down Lock to PointGet and BatchPointGet #15257
Conversation
PointGet and BatchPointGet supports lock by itself, so we can push down the lock operation down to PointGet and BatchPointGet to lock non-exist keys. They also read data from MemBuffer, so UnionScan can be eliminated.
ghost
requested review from
francis0407,
lzmhhh123,
qw4990 and
XuHuaiyu
and removed request for
a team
| iteratePhysicalPlan(p, func(p PhysicalPlan) bool { | ||
| if len(p.Children()) > 1 { | ||
| return false | ||
| } | ||
| switch x := p.(type) { | ||
| case *PointGetPlan: | ||
| pointGet = x | ||
| case *BatchPointGetPlan: | ||
| batchPointGet = x | ||
| case *PhysicalLock: | ||
| physLock = x | ||
| case *PhysicalUnionScan: | ||
| unionScan = x | ||
| } | ||
| return true | ||
| }) |
There was a problem hiding this comment.
How about doing transformPhysicalPlan here to reduce plan tree iteration?
There was a problem hiding this comment.
transform is post-order, iterate is pre-order.
Use pre-order to iterate so we can stop early, and we can not do transform in pre-order.
| return p | ||
| } | ||
| if physLock == nil { | ||
| return p |
There was a problem hiding this comment.
can we eliminate unionScan even if it's a select without for update?
begin pessimistic;
insert t values (8, 8)
explain select c + 1 from t where k = 2 and c = 2; // no need unionScan too?
| // verify that select with project and filter on a non exists key still locks the key. | ||
| tk.MustExec("begin pessimistic") | ||
| tk.MustExec("insert t values (8, 8)") // Make the transaction dirty. | ||
| tk.MustQuery("select c + 1 from t where k = 2 and c = 2 for update").Check(testkit.Rows()) |
There was a problem hiding this comment.
if we change this sql to
select c + 1 from t where k = (2, 3) and c = 2 for update
(3, 3) also be locked after this pr, but mysql seems do the same things - -
There was a problem hiding this comment.
Yes, it is expected, we lock it even if it doesn't pass the filter.
Codecov Report
@@ Coverage Diff @@
## master #15257 +/- ##
===========================================
Coverage ? 80.3457%
===========================================
Files ? 503
Lines ? 132989
Branches ? 0
===========================================
Hits ? 106851
Misses ? 17731
Partials ? 8407 |
lysu
added
status/LGT2
|
/merge |
sre-bot
added
the
status/can-merge
|
/run-all-tests |
|
@coocood merge failed. |
|
/merge |
|
Your auto merge job has been accepted, waiting for 15238 |
|
/run-all-tests |
What problem does this PR solve?
What is changed and how it works?
PointGet and BatchPointGet supports lock by itself, so we can push down
the lock operation down to PointGet and BatchPointGet to lock non-exist keys.
They also read data from MemBuffer, so UnionScan can be eliminated.
By add a function in postOptimize, we can eliminate UnionScan and push down lock to PointGet and BatchPointGet.
Check List
Tests
Code changes
Related changes
Release note