Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitff77d86

Browse files
committed
Fix misbehavior of EvalPlanQual checks with multiple result relations.
The idea of EvalPlanQual is that we replace the query's scan of theresult relation with a single injected tuple, and see if we get atuple out, thereby implying that the injected tuple still passes thequery quals. (In join cases, other relations in the query are stillscanned normally.) This logic was not updated when commit86dc900made it possible for a single DML query plan to have multiple resultrelations, when the query target relation has inheritance or partitionchildren. We replaced the output for the current result relationsuccessfully, but other result relations were still scanned normally;thus, if any other result relation contained a tuple satisfying thequals, we'd think the EPQ check passed, even if it did not pass forthe injected tuple itself. This would lead to update or deleteactions getting performed when they should have been skipped due toa conflicting concurrent update in READ COMMITTED isolation mode.Fix by blocking all sibling result relations from emitting tuplesduring an EvalPlanQual recheck. In the back branches, the fix iscomplicated a bit by the need to not change the size of structEPQState (else we'd have ABI-breaking changes in offsets instruct ModifyTableState). Like the back-patches of3f7836fand4b3e379, add a separately palloc'd struct to avoid that.The logic is the same as in HEAD otherwise.This is only a live bug back to v14 where86dc900 came in.However, I chose to back-patch the test cases further, on thegrounds that this whole area is none too well tested. I skippeddoing so in v11 though because none of the test applied cleanly,and it didn't quite seem worth extra work for a branch with onlysix months to live.Per report from Ante Krešić (via Aleksander Alekseev)Discussion:https://postgr.es/m/CAJ7c6TMBTN3rcz4=AjYhLPD_w3FFT0Wq_C15jxCDn8U4tZnH1g@mail.gmail.com
1 parentcf9de89 commitff77d86

File tree

2 files changed

+132
-12
lines changed

2 files changed

+132
-12
lines changed

‎src/test/isolation/expected/eval-plan-qual.out

Lines changed: 115 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,90 @@ step c1: COMMIT;
842842
step writep3b: <... completed>
843843
step c2: COMMIT;
844844

845+
starting permutation: writep4a writep4b c1 c2 readp
846+
step writep4a: UPDATE p SET c = 4 WHERE c = 0;
847+
step writep4b: UPDATE p SET b = -4 WHERE c = 0; <waiting ...>
848+
step c1: COMMIT;
849+
step writep4b: <... completed>
850+
step c2: COMMIT;
851+
step readp: SELECT tableoid::regclass, ctid, * FROM p;
852+
tableoid|ctid |a|b|c
853+
--------+------+-+-+-
854+
c1 |(0,2) |0|0|1
855+
c1 |(0,3) |0|0|2
856+
c1 |(0,5) |0|1|1
857+
c1 |(0,6) |0|1|2
858+
c1 |(0,8) |0|2|1
859+
c1 |(0,9) |0|2|2
860+
c1 |(0,11)|0|0|4
861+
c1 |(0,12)|0|1|4
862+
c1 |(0,13)|0|2|4
863+
c1 |(0,14)|0|3|4
864+
c2 |(0,2) |1|0|1
865+
c2 |(0,3) |1|0|2
866+
c2 |(0,5) |1|1|1
867+
c2 |(0,6) |1|1|2
868+
c2 |(0,8) |1|2|1
869+
c2 |(0,9) |1|2|2
870+
c2 |(0,11)|1|0|4
871+
c2 |(0,12)|1|1|4
872+
c2 |(0,13)|1|2|4
873+
c2 |(0,14)|1|3|4
874+
c3 |(0,2) |2|0|1
875+
c3 |(0,3) |2|0|2
876+
c3 |(0,5) |2|1|1
877+
c3 |(0,6) |2|1|2
878+
c3 |(0,8) |2|2|1
879+
c3 |(0,9) |2|2|2
880+
c3 |(0,11)|2|0|4
881+
c3 |(0,12)|2|1|4
882+
c3 |(0,13)|2|2|4
883+
c3 |(0,14)|2|3|4
884+
(30 rows)
885+
886+
887+
starting permutation: writep4a deletep4 c1 c2 readp
888+
step writep4a: UPDATE p SET c = 4 WHERE c = 0;
889+
step deletep4: DELETE FROM p WHERE c = 0; <waiting ...>
890+
step c1: COMMIT;
891+
step deletep4: <... completed>
892+
step c2: COMMIT;
893+
step readp: SELECT tableoid::regclass, ctid, * FROM p;
894+
tableoid|ctid |a|b|c
895+
--------+------+-+-+-
896+
c1 |(0,2) |0|0|1
897+
c1 |(0,3) |0|0|2
898+
c1 |(0,5) |0|1|1
899+
c1 |(0,6) |0|1|2
900+
c1 |(0,8) |0|2|1
901+
c1 |(0,9) |0|2|2
902+
c1 |(0,11)|0|0|4
903+
c1 |(0,12)|0|1|4
904+
c1 |(0,13)|0|2|4
905+
c1 |(0,14)|0|3|4
906+
c2 |(0,2) |1|0|1
907+
c2 |(0,3) |1|0|2
908+
c2 |(0,5) |1|1|1
909+
c2 |(0,6) |1|1|2
910+
c2 |(0,8) |1|2|1
911+
c2 |(0,9) |1|2|2
912+
c2 |(0,11)|1|0|4
913+
c2 |(0,12)|1|1|4
914+
c2 |(0,13)|1|2|4
915+
c2 |(0,14)|1|3|4
916+
c3 |(0,2) |2|0|1
917+
c3 |(0,3) |2|0|2
918+
c3 |(0,5) |2|1|1
919+
c3 |(0,6) |2|1|2
920+
c3 |(0,8) |2|2|1
921+
c3 |(0,9) |2|2|2
922+
c3 |(0,11)|2|0|4
923+
c3 |(0,12)|2|1|4
924+
c3 |(0,13)|2|2|4
925+
c3 |(0,14)|2|3|4
926+
(30 rows)
927+
928+
845929
starting permutation: wx2 partiallock c2 c1 read
846930
step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
847931
balance
@@ -1104,22 +1188,41 @@ subid|id
11041188
(1 row)
11051189

11061190

1191+
starting permutation: simplepartupdate conditionalpartupdate c1 c2 read_part
1192+
step simplepartupdate:
1193+
update parttbl set b = b + 10;
1194+
1195+
step conditionalpartupdate:
1196+
update parttbl set c = -c where b < 10;
1197+
<waiting ...>
1198+
step c1: COMMIT;
1199+
step conditionalpartupdate: <... completed>
1200+
step c2: COMMIT;
1201+
step read_part: SELECT * FROM parttbl ORDER BY a, c;
1202+
a| b|c| d
1203+
-+--+-+--
1204+
1|11|1|12
1205+
2|12|2|14
1206+
(2 rows)
1207+
1208+
11071209
starting permutation: simplepartupdate complexpartupdate c1 c2 read_part
11081210
step simplepartupdate:
11091211
update parttbl set b = b + 10;
11101212

11111213
step complexpartupdate:
11121214
with u as (update parttbl set b = b + 1 returning parttbl.*)
1113-
update parttbl set b = u.b + 100 from u;
1215+
update parttblpset b = u.b + 100 from u where p.a = u.a;
11141216
<waiting ...>
11151217
step c1: COMMIT;
11161218
step complexpartupdate: <... completed>
11171219
step c2: COMMIT;
1118-
step read_part: SELECT * FROM parttbl ORDER BY a;
1220+
step read_part: SELECT * FROM parttbl ORDER BY a, c;
11191221
a| b|c| d
11201222
-+--+-+--
11211223
1|12|1|13
1122-
(1 row)
1224+
2|13|2|15
1225+
(2 rows)
11231226

11241227

11251228
starting permutation: simplepartupdate_route1to2 complexpartupdate_route_err1 c1 c2 read_part
@@ -1139,11 +1242,12 @@ step c1: COMMIT;
11391242
step complexpartupdate_route_err1: <... completed>
11401243
ERROR: tuple to be locked was already moved to another partition due to concurrent update
11411244
step c2: COMMIT;
1142-
step read_part: SELECT * FROM parttbl ORDER BY a;
1245+
step read_part: SELECT * FROM parttbl ORDER BY a, c;
11431246
a|b|c|d
11441247
-+-+-+-
11451248
2|1|1|3
1146-
(1 row)
1249+
2|2|2|4
1250+
(2 rows)
11471251

11481252

11491253
starting permutation: simplepartupdate_noroute complexpartupdate_route c1 c2 read_part
@@ -1167,11 +1271,12 @@ a|b|c|d
11671271
(1 row)
11681272

11691273
step c2: COMMIT;
1170-
step read_part: SELECT * FROM parttbl ORDER BY a;
1274+
step read_part: SELECT * FROM parttbl ORDER BY a, c;
11711275
a|b|c|d
11721276
-+-+-+-
11731277
2|2|1|4
1174-
(1 row)
1278+
2|2|2|4
1279+
(2 rows)
11751280

11761281

11771282
starting permutation: simplepartupdate_noroute complexpartupdate_doesnt_route c1 c2 read_part
@@ -1195,9 +1300,10 @@ a|b|c|d
11951300
(1 row)
11961301

11971302
step c2: COMMIT;
1198-
step read_part: SELECT * FROM parttbl ORDER BY a;
1303+
step read_part: SELECT * FROM parttbl ORDER BY a, c;
11991304
a|b|c|d
12001305
-+-+-+-
12011306
1|2|1|3
1202-
(1 row)
1307+
2|2|2|4
1308+
(2 rows)
12031309

‎src/test/isolation/specs/eval-plan-qual.spec

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ setup
3838
dintGENERATEDALWAYSAS (a+b)STORED)PARTITIONBYLIST (a);
3939
CREATETABLEparttbl1PARTITIONOFparttblFORVALUESIN (1);
4040
CREATETABLEparttbl2PARTITIONOFparttblFORVALUESIN (2);
41-
INSERTINTOparttblVALUES (1,1,1);
41+
INSERTINTOparttblVALUES (1,1,1), (2,2,2);
4242

4343
CREATETABLEanother_parttbl (aint,bint,cint)PARTITIONBYLIST (a);
4444
CREATETABLEanother_parttbl1PARTITIONOFanother_parttblFORVALUESIN (1);
@@ -100,11 +100,15 @@ step upsert1{
100100
# when the first updated tuple was in a non-first child table.
101101
# writep2/returningp1 tests a memory allocation issue
102102
# writep3a/writep3b tests updates touching more than one table
103+
# writep4a/writep4b tests a case where matches in another table confused EPQ
104+
# writep4a/deletep4 tests the same case in the DELETE path
103105

106+
stepreadp{SELECTtableoid::regclass,ctid,*FROMp; }
104107
stepreadp1{SELECTtableoid::regclass,ctid,*FROMpWHEREbIN (0,1)ANDc=0FORUPDATE; }
105108
stepwritep1{UPDATEpSETb=-1WHEREa=1ANDb=1ANDc=0; }
106109
stepwritep2{UPDATEpSETb=-bWHEREa=1ANDc=0; }
107110
stepwritep3a{UPDATEpSETb=-bWHEREc=0; }
111+
stepwritep4a{UPDATEpSETc=4WHEREc=0; }
108112
stepc1{COMMIT; }
109113
stepr1{ROLLBACK; }
110114

@@ -208,6 +212,8 @@ step returningp1 {
208212
SELECT*FROMu;
209213
}
210214
stepwritep3b{UPDATEpSETb=-bWHEREc=0; }
215+
stepwritep4b{UPDATEpSETb=-4WHEREc=0; }
216+
stepdeletep4{DELETEFROMpWHEREc=0; }
211217
stepreadforss{
212218
SELECTta.idASta_id,ta.valueASta_value,
213219
(SELECTROW(tb.id,tb.value)
@@ -224,9 +230,14 @@ step updateforcip3{
224230
}
225231
stepwrtwcte{UPDATEtable_aSETvalue='tableAValue2'WHEREid=1; }
226232
stepwrjt{UPDATEjointestSETdata=42WHEREid=7; }
233+
234+
stepconditionalpartupdate{
235+
updateparttblsetc=-cwhereb<10;
236+
}
237+
227238
stepcomplexpartupdate{
228239
withuas (updateparttblsetb=b+1returningparttbl.*)
229-
updateparttblsetb=u.b+100fromu;
240+
updateparttblpsetb=u.b+100fromuwherep.a=u.a;
230241
}
231242

232243
stepcomplexpartupdate_route_err1 {
@@ -275,7 +286,7 @@ setup{ BEGIN ISOLATION LEVEL READ COMMITTED; SET client_min_messages = 'WARNIN
275286
stepread{SELECT*FROMaccountsORDERBYaccountid; }
276287
stepread_ext{SELECT*FROMaccounts_extORDERBYaccountid; }
277288
stepread_a{SELECT*FROMtable_aORDERBYid; }
278-
stepread_part{SELECT*FROMparttblORDERBYa; }
289+
stepread_part{SELECT*FROMparttblORDERBYa,c; }
279290

280291
# this test exercises EvalPlanQual with a CTE, cf bug #14328
281292
stepreadwcte{
@@ -345,6 +356,8 @@ permutation upsert1 upsert2 c1 c2 read
345356
permutationreadp1writep1readp2c1c2
346357
permutationwritep2returningp1c1c2
347358
permutationwritep3awritep3bc1c2
359+
permutationwritep4awritep4bc1c2readp
360+
permutationwritep4adeletep4c1c2readp
348361
permutationwx2partiallockc2c1read
349362
permutationwx2lockwithvaluesc2c1read
350363
permutationwx2_extpartiallock_extc2c1read_ext
@@ -356,6 +369,7 @@ permutation wrjt selectjoinforupdate c2 c1
356369
permutationwrjtselectresultforupdatec2c1
357370
permutationwrtwctemultireadwctec1c2
358371

372+
permutationsimplepartupdateconditionalpartupdatec1c2read_part
359373
permutationsimplepartupdatecomplexpartupdatec1c2read_part
360374
permutationsimplepartupdate_route1to2complexpartupdate_route_err1c1c2read_part
361375
permutationsimplepartupdate_noroutecomplexpartupdate_routec1c2read_part

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp