Skip to content

Commit ef1e125

Browse files
alvherrealvherre
committed
Check default partitions constraints while descending
Partitioning tuple route code assumes that the partition chosen while descending the partition hierarchy is always the correct one. This is true except when the partition is the default partition and another partition has been added concurrently: the partition constraint changes and we don't recheck it. This can lead to tuples mistakenly being added to the default partition that should have been rejected. Fix by rechecking the default partition constraint while descending the hierarchy. An isolation test based on the reproduction steps described by Hao Wu (with tweaks for extra coverage) is included. Backpatch to 12, where this bug came in with 898e5e3. Reported by: Hao Wu <hawu@vmware.com> Author: Amit Langote <amitlangote09@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA@mail.gmail.com Discussion: https://postgr.es/m/DM5PR0501MB3910E97A9EDFB4C775CF3D75A42F0@DM5PR0501MB3910.namprd05.prod.outlook.com
1 parent 87483c7 commit ef1e125

File tree

4 files changed

+195
-25
lines changed

4 files changed

+195
-25
lines changed

‎src/backend/executor/execPartition.c

Lines changed: 102 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
* PartitionDispatchData->indexes for details on how this array is
5252
* indexed.
5353
*
54+
* nonleaf_partitions
55+
* Array of 'max_dispatch' elements containing pointers to fake
56+
* ResultRelInfo objects for nonleaf partitions, useful for checking
57+
* the partition constraint.
58+
*
5459
* num_dispatch
5560
* The current number of items stored in the 'partition_dispatch_info'
5661
* array. Also serves as the index of the next free array element for
@@ -89,6 +94,7 @@ struct PartitionTupleRouting
8994
{
9095
Relation partition_root;
9196
PartitionDispatch *partition_dispatch_info;
97+
ResultRelInfo **nonleaf_partitions;
9298
int num_dispatch;
9399
int max_dispatch;
94100
ResultRelInfo **partitions;
@@ -280,9 +286,11 @@ ExecFindPartition(ModifyTableState *mtstate,
280286
PartitionDispatch dispatch;
281287
PartitionDesc partdesc;
282288
ExprContext *ecxt = GetPerTupleExprContext(estate);
283-
TupleTableSlot *ecxt_scantuple_old = ecxt->ecxt_scantuple;
289+
TupleTableSlot *ecxt_scantuple_saved = ecxt->ecxt_scantuple;
290+
TupleTableSlot *rootslot = slot;
284291
TupleTableSlot *myslot = NULL;
285292
MemoryContext oldcxt;
293+
ResultRelInfo *rri = NULL;
286294

287295
/* use per-tuple context here to avoid leaking memory */
288296
oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
@@ -296,27 +304,15 @@ ExecFindPartition(ModifyTableState *mtstate,
296304

297305
/* start with the root partitioned table */
298306
dispatch = pd[0];
299-
while (true)
307+
while (dispatch != NULL)
300308
{
301-
AttrNumber *map = dispatch->tupmap;
302309
int partidx = -1;
303310

304311
CHECK_FOR_INTERRUPTS();
305312

306313
rel = dispatch->reldesc;
307314
partdesc = dispatch->partdesc;
308315

309-
/*
310-
* Convert the tuple to this parent's layout, if different from the
311-
* current relation.
312-
*/
313-
myslot = dispatch->tupslot;
314-
if (myslot != NULL)
315-
{
316-
Assert(map != NULL);
317-
slot = execute_attr_map_slot(map, slot, myslot);
318-
}
319-
320316
/*
321317
* Extract partition key from tuple. Expression evaluation machinery
322318
* that FormPartitionKeyDatum() invokes expects ecxt_scantuple to
@@ -351,11 +347,9 @@ ExecFindPartition(ModifyTableState *mtstate,
351347

352348
if (partdesc->is_leaf[partidx])
353349
{
354-
ResultRelInfo *rri;
355-
356350
/*
357-
* Look to see if we've already got a ResultRelInfo for this
358-
* partition.
351+
* We've reached the leaf -- hurray, we're done. Look to see if
352+
* we've already got a ResultRelInfo for this partition.
359353
*/
360354
if (likely(dispatch->indexes[partidx] >= 0))
361355
{
@@ -399,14 +393,10 @@ ExecFindPartition(ModifyTableState *mtstate,
399393
dispatch,
400394
rootResultRelInfo, partidx);
401395
}
396+
Assert(rri != NULL);
402397

403-
/* Release the tuple in the lowest parent's dedicated slot. */
404-
if (slot == myslot)
405-
ExecClearTuple(myslot);
406-
407-
MemoryContextSwitchTo(oldcxt);
408-
ecxt->ecxt_scantuple = ecxt_scantuple_old;
409-
return rri;
398+
/* Signal to terminate the loop */
399+
dispatch = NULL;
410400
}
411401
else
412402
{
@@ -418,6 +408,8 @@ ExecFindPartition(ModifyTableState *mtstate,
418408
/* Already built. */
419409
Assert(dispatch->indexes[partidx] < proute->num_dispatch);
420410

411+
rri = proute->nonleaf_partitions[dispatch->indexes[partidx]];
412+
421413
/*
422414
* Move down to the next partition level and search again
423415
* until we find a leaf partition that matches this tuple
@@ -439,10 +431,75 @@ ExecFindPartition(ModifyTableState *mtstate,
439431
dispatch, partidx);
440432
Assert(dispatch->indexes[partidx] >= 0 &&
441433
dispatch->indexes[partidx] < proute->num_dispatch);
434+
435+
rri = proute->nonleaf_partitions[dispatch->indexes[partidx]];
442436
dispatch = subdispatch;
443437
}
438+
439+
/*
440+
* Convert the tuple to the new parent's layout, if different from
441+
* the previous parent.
442+
*/
443+
if (dispatch->tupslot)
444+
{
445+
AttrNumber *map = dispatch->tupmap;
446+
TupleTableSlot *tempslot = myslot;
447+
448+
myslot = dispatch->tupslot;
449+
slot = execute_attr_map_slot(map, slot, myslot);
450+
451+
if (tempslot != NULL)
452+
ExecClearTuple(tempslot);
453+
}
454+
}
455+
456+
/*
457+
* If this partition is the default one, we must check its partition
458+
* constraint now, which may have changed concurrently due to
459+
* partitions being added to the parent.
460+
*
461+
* (We do this here, and do not rely on ExecInsert doing it, because
462+
* we don't want to miss doing it for non-leaf partitions.)
463+
*/
464+
if (partidx == partdesc->boundinfo->default_index)
465+
{
466+
PartitionRoutingInfo *partrouteinfo = rri->ri_PartitionInfo;
467+
468+
/*
469+
* The tuple must match the partition's layout for the constraint
470+
* expression to be evaluated successfully. If the partition is
471+
* sub-partitioned, that would already be the case due to the code
472+
* above, but for a leaf partition the tuple still matches the
473+
* parent's layout.
474+
*
475+
* Note that we have a map to convert from root to current
476+
* partition, but not from immediate parent to current partition.
477+
* So if we have to convert, do it from the root slot; if not, use
478+
* the root slot as-is.
479+
*/
480+
if (partrouteinfo)
481+
{
482+
TupleConversionMap *map = partrouteinfo->pi_RootToPartitionMap;
483+
484+
if (map)
485+
slot = execute_attr_map_slot(map->attrMap, rootslot,
486+
partrouteinfo->pi_PartitionTupleSlot);
487+
else
488+
slot = rootslot;
489+
}
490+
491+
ExecPartitionCheck(rri, slot, estate, true);
444492
}
445493
}
494+
495+
/* Release the tuple in the lowest parent's dedicated slot. */
496+
if (myslot != NULL)
497+
ExecClearTuple(myslot);
498+
/* and restore ecxt's scantuple */
499+
ecxt->ecxt_scantuple = ecxt_scantuple_saved;
500+
MemoryContextSwitchTo(oldcxt);
501+
502+
return rri;
446503
}
447504

448505
/*
@@ -1071,17 +1128,37 @@ ExecInitPartitionDispatchInfo(EState *estate,
10711128
proute->max_dispatch = 4;
10721129
proute->partition_dispatch_info = (PartitionDispatch *)
10731130
palloc(sizeof(PartitionDispatch) * proute->max_dispatch);
1131+
proute->nonleaf_partitions = (ResultRelInfo **)
1132+
palloc(sizeof(ResultRelInfo *) * proute->max_dispatch);
10741133
}
10751134
else
10761135
{
10771136
proute->max_dispatch *= 2;
10781137
proute->partition_dispatch_info = (PartitionDispatch *)
10791138
repalloc(proute->partition_dispatch_info,
10801139
sizeof(PartitionDispatch) * proute->max_dispatch);
1140+
proute->nonleaf_partitions = (ResultRelInfo **)
1141+
repalloc(proute->nonleaf_partitions,
1142+
sizeof(ResultRelInfo *) * proute->max_dispatch);
10811143
}
10821144
}
10831145
proute->partition_dispatch_info[dispatchidx] = pd;
10841146

1147+
/*
1148+
* If setting up a PartitionDispatch for a sub-partitioned table, we may
1149+
* also need a minimally valid ResultRelInfo for checking the partition
1150+
* constraint later; set that up now.
1151+
*/
1152+
if (parent_pd)
1153+
{
1154+
ResultRelInfo *rri = makeNode(ResultRelInfo);
1155+
1156+
InitResultRelInfo(rri, rel, 1, proute->partition_root, 0);
1157+
proute->nonleaf_partitions[dispatchidx] = rri;
1158+
}
1159+
else
1160+
proute->nonleaf_partitions[dispatchidx] = NULL;
1161+
10851162
/*
10861163
* Finally, if setting up a PartitionDispatch for a sub-partitioned table,
10871164
* install a downlink in the parent to allow quick descent.

‎src/test/isolation/expected/partition-concurrent-attach.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s1b s1a s2b s2i s1c s2c s2s
4+
step s1b: begin;
5+
step s1a: alter table tpart attach partition tpart_2 for values from (100) to (200);
6+
step s2b: begin;
7+
step s2i: insert into tpart values (110,'xxx'), (120, 'yyy'), (150, 'zzz'); <waiting ...>
8+
step s1c: commit;
9+
step s2i: <... completed>
10+
error in steps s1c s2i: ERROR: new row for relation "tpart_default" violates partition constraint
11+
step s2c: commit;
12+
step s2s: select tableoid::regclass, * from tpart;
13+
tableoid i j
14+
15+
tpart_2 110 xxx
16+
tpart_2 120 yyy
17+
tpart_2 150 zzz
18+
19+
starting permutation: s1b s1a s2b s2i2 s1c s2c s2s
20+
step s1b: begin;
21+
step s1a: alter table tpart attach partition tpart_2 for values from (100) to (200);
22+
step s2b: begin;
23+
step s2i2: insert into tpart_default (i, j) values (110, 'xxx'), (120, 'yyy'), (150, 'zzz'); <waiting ...>
24+
step s1c: commit;
25+
step s2i2: <... completed>
26+
error in steps s1c s2i2: ERROR: new row for relation "tpart_default" violates partition constraint
27+
step s2c: commit;
28+
step s2s: select tableoid::regclass, * from tpart;
29+
tableoid i j
30+
31+
tpart_2 110 xxx
32+
tpart_2 120 yyy
33+
tpart_2 150 zzz
34+
35+
starting permutation: s1b s2b s2i s1a s2c s1c s2s
36+
step s1b: begin;
37+
step s2b: begin;
38+
step s2i: insert into tpart values (110,'xxx'), (120, 'yyy'), (150, 'zzz');
39+
step s1a: alter table tpart attach partition tpart_2 for values from (100) to (200); <waiting ...>
40+
step s2c: commit;
41+
step s1a: <... completed>
42+
error in steps s2c s1a: ERROR: updated partition constraint for default partition would be violated by some row
43+
step s1c: commit;
44+
step s2s: select tableoid::regclass, * from tpart;
45+
tableoid i j
46+
47+
tpart_default_default110 xxx
48+
tpart_default_default120 yyy
49+
tpart_default_default150 zzz

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ test: vacuum-skip-locked
8080
test: predicate-hash
8181
test: predicate-gist
8282
test: predicate-gin
83+
test: partition-concurrent-attach
8384
test: partition-key-update-1
8485
test: partition-key-update-2
8586
test: partition-key-update-3

‎src/test/isolation/specs/partition-concurrent-attach.spec

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Verify that default partition constraint is enforced correctly
2+
# in light of partitions being added concurrently to its parent
3+
setup {
4+
drop table if exists tpart;
5+
create table tpart(i int, j text) partition by range(i);
6+
create table tpart_1(like tpart);
7+
create table tpart_2(like tpart);
8+
create table tpart_default (a int, j text, i int) partition by list (j);
9+
create table tpart_default_default (a int, i int, b int, j text);
10+
alter table tpart_default_default drop b;
11+
alter table tpart_default attach partition tpart_default_default default;
12+
alter table tpart_default drop a;
13+
alter table tpart attach partition tpart_default default;
14+
alter table tpart attach partition tpart_1 for values from(0) to (100);
15+
insert into tpart_2 values (110,'xxx'), (120, 'yyy'), (150, 'zzz');
16+
}
17+
18+
session "s1"
19+
step "s1b" { begin; }
20+
step "s1a" { alter table tpart attach partition tpart_2 for values from (100) to (200); }
21+
step "s1c" { commit; }
22+
23+
session "s2"
24+
step "s2b" { begin; }
25+
step "s2i" { insert into tpart values (110,'xxx'), (120, 'yyy'), (150, 'zzz'); }
26+
step "s2i2" { insert into tpart_default (i, j) values (110, 'xxx'), (120, 'yyy'), (150, 'zzz'); }
27+
step "s2c" { commit; }
28+
step "s2s" { select tableoid::regclass, * from tpart; }
29+
30+
teardown { drop table tpart; }
31+
32+
# insert into tpart by s2 which routes to tpart_default due to not seeing
33+
# concurrently added tpart_2 should fail, because the partition constraint
34+
# of tpart_default would have changed due to tpart_2 having been added
35+
permutation "s1b" "s1a" "s2b" "s2i" "s1c" "s2c" "s2s"
36+
37+
# similar to above, but now insert into sub-partitioned tpart_default
38+
permutation "s1b" "s1a" "s2b" "s2i2" "s1c" "s2c" "s2s"
39+
40+
# reverse: now the insert into tpart_default by s2 occurs first followed by
41+
# attach in s1, which should fail when it scans the leaf default partition
42+
# find the violating rows
43+
permutation "s1b" "s2b" "s2i" "s1a" "s2c" "s1c" "s2s"

0 commit comments

Comments
 (0)