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

Commit6666ee4

Browse files
committed
Fix state reversal after partition tuple routing
We make some changes to ModifyTableState and the EState it uses wheneverwe route tuples to partitions; but we weren't restoring properly in allcases, possibly causing crashes when partitions with different tupledescriptors are targeted by tuples inserted in the same command.Refactor some code, creating ExecPrepareTupleRouting, to encapsulate theneeded state changing logic, and have it invoked one level above itscurrent place (ie. put it in ExecModifyTable instead of ExecInsert);this makes it all more readable.Add a test case to exercise this.We don't support having views as partitions; and since only views canhave INSTEAD OF triggers, there is no point in testing for INSTEAD OFwhen processing insertions into a partitioned table. Remove code thatappears to support this (but which is actually never relevant.)In passing, fix location of some very confusing comments inModifyTableState.Reported-by: Amit LangoteAuthor: Etsuro Fujita, Amit LangoteDiscussion: https://postgr/es/m/0473bf5c-57b1-f1f7-3d58-455c2230bc5f@lab.ntt.co.jp
1 parentc596fad commit6666ee4

File tree

5 files changed

+188
-123
lines changed

5 files changed

+188
-123
lines changed

‎src/backend/commands/copy.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2633,13 +2633,12 @@ CopyFrom(CopyState cstate)
26332633
if (cstate->transition_capture!=NULL)
26342634
{
26352635
if (resultRelInfo->ri_TrigDesc&&
2636-
(resultRelInfo->ri_TrigDesc->trig_insert_before_row||
2637-
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
2636+
resultRelInfo->ri_TrigDesc->trig_insert_before_row)
26382637
{
26392638
/*
2640-
* If there are any BEFOREor INSTEADtriggers on the
2641-
*partition,we'll have to be ready to convert their
2642-
*result back totuplestore format.
2639+
* If there are any BEFORE triggers on the partition,
2640+
* we'll have to be ready to convert their result back to
2641+
* tuplestore format.
26432642
*/
26442643
cstate->transition_capture->tcs_original_insert_tuple=NULL;
26452644
cstate->transition_capture->tcs_map=
@@ -2768,12 +2767,13 @@ CopyFrom(CopyState cstate)
27682767
* tuples inserted by an INSERT command.
27692768
*/
27702769
processed++;
2770+
}
27712771

2772-
if (saved_resultRelInfo)
2773-
{
2774-
resultRelInfo=saved_resultRelInfo;
2775-
estate->es_result_relation_info=resultRelInfo;
2776-
}
2772+
/* Restore the saved ResultRelInfo */
2773+
if (saved_resultRelInfo)
2774+
{
2775+
resultRelInfo=saved_resultRelInfo;
2776+
estate->es_result_relation_info=resultRelInfo;
27772777
}
27782778
}
27792779

‎src/backend/executor/nodeModifyTable.c

Lines changed: 121 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ static bool ExecOnConflictUpdate(ModifyTableState *mtstate,
6262
EState*estate,
6363
boolcanSetTag,
6464
TupleTableSlot**returning);
65+
staticTupleTableSlot*ExecPrepareTupleRouting(ModifyTableState*mtstate,
66+
EState*estate,
67+
PartitionTupleRouting*proute,
68+
ResultRelInfo*targetRelInfo,
69+
TupleTableSlot*slot);
6570
staticResultRelInfo*getTargetResultRelInfo(ModifyTableState*node);
6671
staticvoidExecSetupChildParentMapForTcs(ModifyTableState*mtstate);
6772
staticvoidExecSetupChildParentMapForSubplan(ModifyTableState*mtstate);
@@ -265,7 +270,6 @@ ExecInsert(ModifyTableState *mtstate,
265270
{
266271
HeapTupletuple;
267272
ResultRelInfo*resultRelInfo;
268-
ResultRelInfo*saved_resultRelInfo=NULL;
269273
RelationresultRelationDesc;
270274
OidnewId;
271275
List*recheckIndexes=NIL;
@@ -282,100 +286,6 @@ ExecInsert(ModifyTableState *mtstate,
282286
* get information on the (current) result relation
283287
*/
284288
resultRelInfo=estate->es_result_relation_info;
285-
286-
/* Determine the partition to heap_insert the tuple into */
287-
if (mtstate->mt_partition_tuple_routing)
288-
{
289-
intleaf_part_index;
290-
PartitionTupleRouting*proute=mtstate->mt_partition_tuple_routing;
291-
292-
/*
293-
* Away we go ... If we end up not finding a partition after all,
294-
* ExecFindPartition() does not return and errors out instead.
295-
* Otherwise, the returned value is to be used as an index into arrays
296-
* proute->partitions[] and proute->partition_tupconv_maps[] that will
297-
* get us the ResultRelInfo and TupleConversionMap for the partition,
298-
* respectively.
299-
*/
300-
leaf_part_index=ExecFindPartition(resultRelInfo,
301-
proute->partition_dispatch_info,
302-
slot,
303-
estate);
304-
Assert(leaf_part_index >=0&&
305-
leaf_part_index<proute->num_partitions);
306-
307-
/*
308-
* Save the old ResultRelInfo and switch to the one corresponding to
309-
* the selected partition. (We might need to initialize it first.)
310-
*/
311-
saved_resultRelInfo=resultRelInfo;
312-
resultRelInfo=proute->partitions[leaf_part_index];
313-
if (resultRelInfo==NULL)
314-
{
315-
resultRelInfo=ExecInitPartitionInfo(mtstate,
316-
saved_resultRelInfo,
317-
proute,estate,
318-
leaf_part_index);
319-
Assert(resultRelInfo!=NULL);
320-
}
321-
322-
/* We do not yet have a way to insert into a foreign partition */
323-
if (resultRelInfo->ri_FdwRoutine)
324-
ereport(ERROR,
325-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
326-
errmsg("cannot route inserted tuples to a foreign table")));
327-
328-
/* For ExecInsertIndexTuples() to work on the partition's indexes */
329-
estate->es_result_relation_info=resultRelInfo;
330-
331-
/*
332-
* If we're capturing transition tuples, we might need to convert from
333-
* the partition rowtype to parent rowtype.
334-
*/
335-
if (mtstate->mt_transition_capture!=NULL)
336-
{
337-
if (resultRelInfo->ri_TrigDesc&&
338-
(resultRelInfo->ri_TrigDesc->trig_insert_before_row||
339-
resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
340-
{
341-
/*
342-
* If there are any BEFORE or INSTEAD triggers on the
343-
* partition, we'll have to be ready to convert their result
344-
* back to tuplestore format.
345-
*/
346-
mtstate->mt_transition_capture->tcs_original_insert_tuple=NULL;
347-
348-
mtstate->mt_transition_capture->tcs_map=
349-
TupConvMapForLeaf(proute,saved_resultRelInfo,
350-
leaf_part_index);
351-
}
352-
else
353-
{
354-
/*
355-
* Otherwise, just remember the original unconverted tuple, to
356-
* avoid a needless round trip conversion.
357-
*/
358-
mtstate->mt_transition_capture->tcs_original_insert_tuple=tuple;
359-
mtstate->mt_transition_capture->tcs_map=NULL;
360-
}
361-
}
362-
if (mtstate->mt_oc_transition_capture!=NULL)
363-
{
364-
mtstate->mt_oc_transition_capture->tcs_map=
365-
TupConvMapForLeaf(proute,saved_resultRelInfo,
366-
leaf_part_index);
367-
}
368-
369-
/*
370-
* We might need to convert from the parent rowtype to the partition
371-
* rowtype.
372-
*/
373-
tuple=ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[leaf_part_index],
374-
tuple,
375-
proute->partition_tuple_slot,
376-
&slot);
377-
}
378-
379289
resultRelationDesc=resultRelInfo->ri_RelationDesc;
380290

381291
/*
@@ -495,7 +405,7 @@ ExecInsert(ModifyTableState *mtstate,
495405
* No need though if the tuple has been routed, and a BR trigger
496406
* doesn't exist.
497407
*/
498-
if (saved_resultRelInfo!=NULL&&
408+
if (resultRelInfo->ri_PartitionRoot!=NULL&&
499409
!(resultRelInfo->ri_TrigDesc&&
500410
resultRelInfo->ri_TrigDesc->trig_insert_before_row))
501411
check_partition_constr= false;
@@ -686,9 +596,6 @@ ExecInsert(ModifyTableState *mtstate,
686596
if (resultRelInfo->ri_projectReturning)
687597
result=ExecProcessReturning(resultRelInfo,slot,planSlot);
688598

689-
if (saved_resultRelInfo)
690-
estate->es_result_relation_info=saved_resultRelInfo;
691-
692599
returnresult;
693600
}
694601

@@ -1209,27 +1116,22 @@ lreplace:;
12091116
proute->root_tuple_slot,
12101117
&slot);
12111118

1212-
1213-
/*
1214-
* For ExecInsert(), make it look like we are inserting into the
1215-
* root.
1216-
*/
1119+
/* Prepare for tuple routing */
12171120
Assert(mtstate->rootResultRelInfo!=NULL);
1218-
estate->es_result_relation_info=mtstate->rootResultRelInfo;
1121+
slot=ExecPrepareTupleRouting(mtstate,estate,proute,
1122+
mtstate->rootResultRelInfo,slot);
12191123

12201124
ret_slot=ExecInsert(mtstate,slot,planSlot,NULL,
12211125
ONCONFLICT_NONE,estate,canSetTag);
12221126

1223-
/*
1224-
* Revert back the active result relation and the active
1225-
* transition capture map that we changed above.
1226-
*/
1127+
/* Revert ExecPrepareTupleRouting's node change. */
12271128
estate->es_result_relation_info=resultRelInfo;
12281129
if (mtstate->mt_transition_capture)
12291130
{
12301131
mtstate->mt_transition_capture->tcs_original_insert_tuple=NULL;
12311132
mtstate->mt_transition_capture->tcs_map=saved_tcs_map;
12321133
}
1134+
12331135
returnret_slot;
12341136
}
12351137

@@ -1710,6 +1612,108 @@ ExecSetupTransitionCaptureState(ModifyTableState *mtstate, EState *estate)
17101612
}
17111613
}
17121614

1615+
/*
1616+
* ExecPrepareTupleRouting --- prepare for routing one tuple
1617+
*
1618+
* Determine the partition in which the tuple in slot is to be inserted,
1619+
* and modify mtstate and estate to prepare for it.
1620+
*
1621+
* Caller must revert the estate changes after executing the insertion!
1622+
* In mtstate, transition capture changes may also need to be reverted.
1623+
*
1624+
* Returns a slot holding the tuple of the partition rowtype.
1625+
*/
1626+
staticTupleTableSlot*
1627+
ExecPrepareTupleRouting(ModifyTableState*mtstate,
1628+
EState*estate,
1629+
PartitionTupleRouting*proute,
1630+
ResultRelInfo*targetRelInfo,
1631+
TupleTableSlot*slot)
1632+
{
1633+
intpartidx;
1634+
ResultRelInfo*partrel;
1635+
HeapTupletuple;
1636+
1637+
/*
1638+
* Determine the target partition. If ExecFindPartition does not find
1639+
* a partition after all, it doesn't return here; otherwise, the returned
1640+
* value is to be used as an index into the arrays for the ResultRelInfo
1641+
* and TupleConversionMap for the partition.
1642+
*/
1643+
partidx=ExecFindPartition(targetRelInfo,
1644+
proute->partition_dispatch_info,
1645+
slot,
1646+
estate);
1647+
Assert(partidx >=0&&partidx<proute->num_partitions);
1648+
1649+
/*
1650+
* Get the ResultRelInfo corresponding to the selected partition; if not
1651+
* yet there, initialize it.
1652+
*/
1653+
partrel=proute->partitions[partidx];
1654+
if (partrel==NULL)
1655+
partrel=ExecInitPartitionInfo(mtstate,targetRelInfo,
1656+
proute,estate,
1657+
partidx);
1658+
1659+
/* We do not yet have a way to insert into a foreign partition */
1660+
if (partrel->ri_FdwRoutine)
1661+
ereport(ERROR,
1662+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1663+
errmsg("cannot route inserted tuples to a foreign table")));
1664+
1665+
/*
1666+
* Make it look like we are inserting into the partition.
1667+
*/
1668+
estate->es_result_relation_info=partrel;
1669+
1670+
/* Get the heap tuple out of the given slot. */
1671+
tuple=ExecMaterializeSlot(slot);
1672+
1673+
/*
1674+
* If we're capturing transition tuples, we might need to convert from the
1675+
* partition rowtype to parent rowtype.
1676+
*/
1677+
if (mtstate->mt_transition_capture!=NULL)
1678+
{
1679+
if (partrel->ri_TrigDesc&&
1680+
partrel->ri_TrigDesc->trig_insert_before_row)
1681+
{
1682+
/*
1683+
* If there are any BEFORE triggers on the partition, we'll have
1684+
* to be ready to convert their result back to tuplestore format.
1685+
*/
1686+
mtstate->mt_transition_capture->tcs_original_insert_tuple=NULL;
1687+
mtstate->mt_transition_capture->tcs_map=
1688+
TupConvMapForLeaf(proute,targetRelInfo,partidx);
1689+
}
1690+
else
1691+
{
1692+
/*
1693+
* Otherwise, just remember the original unconverted tuple, to
1694+
* avoid a needless round trip conversion.
1695+
*/
1696+
mtstate->mt_transition_capture->tcs_original_insert_tuple=tuple;
1697+
mtstate->mt_transition_capture->tcs_map=NULL;
1698+
}
1699+
}
1700+
if (mtstate->mt_oc_transition_capture!=NULL)
1701+
{
1702+
mtstate->mt_oc_transition_capture->tcs_map=
1703+
TupConvMapForLeaf(proute,targetRelInfo,partidx);
1704+
}
1705+
1706+
/*
1707+
* Convert the tuple, if necessary.
1708+
*/
1709+
ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[partidx],
1710+
tuple,
1711+
proute->partition_tuple_slot,
1712+
&slot);
1713+
1714+
returnslot;
1715+
}
1716+
17131717
/*
17141718
* Initialize the child-to-root tuple conversion map array for UPDATE subplans.
17151719
*
@@ -1846,6 +1850,7 @@ static TupleTableSlot *
18461850
ExecModifyTable(PlanState*pstate)
18471851
{
18481852
ModifyTableState*node=castNode(ModifyTableState,pstate);
1853+
PartitionTupleRouting*proute=node->mt_partition_tuple_routing;
18491854
EState*estate=node->ps.state;
18501855
CmdTypeoperation=node->operation;
18511856
ResultRelInfo*saved_resultRelInfo;
@@ -2051,9 +2056,16 @@ ExecModifyTable(PlanState *pstate)
20512056
switch (operation)
20522057
{
20532058
caseCMD_INSERT:
2059+
/* Prepare for tuple routing if needed. */
2060+
if (proute)
2061+
slot=ExecPrepareTupleRouting(node,estate,proute,
2062+
resultRelInfo,slot);
20542063
slot=ExecInsert(node,slot,planSlot,
20552064
node->mt_arbiterindexes,node->mt_onconflict,
20562065
estate,node->canSetTag);
2066+
/* Revert ExecPrepareTupleRouting's state change. */
2067+
if (proute)
2068+
estate->es_result_relation_info=resultRelInfo;
20572069
break;
20582070
caseCMD_UPDATE:
20592071
slot=ExecUpdate(node,tupleid,oldtuple,slot,planSlot,

‎src/include/nodes/execnodes.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -995,14 +995,18 @@ typedef struct ModifyTableState
995995
TupleTableSlot*mt_existing;/* slot to store existing target tuple in */
996996
List*mt_excludedtlist;/* the excluded pseudo relation's tlist */
997997
TupleTableSlot*mt_conflproj;/* CONFLICT ... SET ... projection target */
998-
structPartitionTupleRouting*mt_partition_tuple_routing;
998+
999999
/* Tuple-routing support info */
1000-
structTransitionCaptureState*mt_transition_capture;
1000+
structPartitionTupleRouting*mt_partition_tuple_routing;
1001+
10011002
/* controls transition table population for specified operation */
1002-
structTransitionCaptureState*mt_oc_transition_capture;
1003+
structTransitionCaptureState*mt_transition_capture;
1004+
10031005
/* controls transition table population for INSERT...ON CONFLICT UPDATE */
1004-
TupleConversionMap**mt_per_subplan_tupconv_maps;
1006+
structTransitionCaptureState*mt_oc_transition_capture;
1007+
10051008
/* Per plan map for tuple conversion from child to root */
1009+
TupleConversionMap**mt_per_subplan_tupconv_maps;
10061010
}ModifyTableState;
10071011

10081012
/* ----------------

‎src/test/regress/expected/insert.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,32 @@ drop role regress_coldesc_role;
748748
drop table inserttest3;
749749
drop table brtrigpartcon;
750750
drop function brtrigpartcon1trigf();
751+
-- check that "do nothing" BR triggers work with tuple-routing (this checks
752+
-- that estate->es_result_relation_info is appropriately set/reset for each
753+
-- routed tuple)
754+
create table donothingbrtrig_test (a int, b text) partition by list (a);
755+
create table donothingbrtrig_test1 (b text, a int);
756+
create table donothingbrtrig_test2 (c text, b text, a int);
757+
alter table donothingbrtrig_test2 drop column c;
758+
create or replace function donothingbrtrig_func() returns trigger as $$begin raise notice 'b: %', new.b; return NULL; end$$ language plpgsql;
759+
create trigger donothingbrtrig1 before insert on donothingbrtrig_test1 for each row execute procedure donothingbrtrig_func();
760+
create trigger donothingbrtrig2 before insert on donothingbrtrig_test2 for each row execute procedure donothingbrtrig_func();
761+
alter table donothingbrtrig_test attach partition donothingbrtrig_test1 for values in (1);
762+
alter table donothingbrtrig_test attach partition donothingbrtrig_test2 for values in (2);
763+
insert into donothingbrtrig_test values (1, 'foo'), (2, 'bar');
764+
NOTICE: b: foo
765+
NOTICE: b: bar
766+
copy donothingbrtrig_test from stdout;
767+
NOTICE: b: baz
768+
NOTICE: b: qux
769+
select tableoid::regclass, * from donothingbrtrig_test;
770+
tableoid | a | b
771+
----------+---+---
772+
(0 rows)
773+
774+
-- cleanup
775+
drop table donothingbrtrig_test;
776+
drop function donothingbrtrig_func();
751777
-- check multi-column range partitioning with minvalue/maxvalue constraints
752778
create table mcrparted (a text, b int) partition by range(a, b);
753779
create table mcrparted1_lt_b partition of mcrparted for values from (minvalue, minvalue) to ('b', minvalue);

0 commit comments

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