Skip to content

Commit c0a8ae7

Browse files
robertmhaasrobertmhaas
committed
Fix reporting of violations in ExecConstraints, again.
We decided in f1b4c77 to pass the original slot to ExecConstraints(), but that breaks when there are BEFORE ROW triggers involved. So we need to do reverse-map the tuples back to the original descriptor instead, as Amit originally proposed. Amit Langote, reviewed by Ashutosh Bapat. One overlooked comment fixed by me. Discussion: http://postgr.es/m/b3a17254-6849-e542-2353-bde4e880b6a4@lab.ntt.co.jp
1 parent 244f1c8 commit c0a8ae7

File tree

7 files changed

+92
-28
lines changed

7 files changed

+92
-28
lines changed

‎src/backend/commands/copy.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2506,8 +2506,7 @@ CopyFrom(CopyState cstate)
25062506

25072507
for (;;)
25082508
{
2509-
TupleTableSlot *slot,
2510-
*oldslot;
2509+
TupleTableSlot *slot;
25112510
bool skip_tuple;
25122511
Oid loaded_oid = InvalidOid;
25132512

@@ -2549,7 +2548,6 @@ CopyFrom(CopyState cstate)
25492548
ExecStoreTuple(tuple, slot, InvalidBuffer, false);
25502549

25512550
/* Determine the partition to heap_insert the tuple into */
2552-
oldslot = slot;
25532551
if (cstate->partition_dispatch_info)
25542552
{
25552553
int leaf_part_index;
@@ -2651,7 +2649,7 @@ CopyFrom(CopyState cstate)
26512649
/* Check the constraints of the tuple */
26522650
if (cstate->rel->rd_att->constr ||
26532651
resultRelInfo->ri_PartitionCheck)
2654-
ExecConstraints(resultRelInfo, slot, oldslot, estate);
2652+
ExecConstraints(resultRelInfo, slot, estate);
26552653

26562654
if (useHeapMultiInsert)
26572655
{

‎src/backend/executor/execMain.c

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,14 +1824,12 @@ ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
18241824
* the partition constraint, if any.
18251825
*
18261826
* Note: 'slot' contains the tuple to check the constraints of, which may
1827-
* have been converted from the original input tuple after tuple routing,
1828-
* while 'orig_slot' contains the original tuple to be shown in the message,
1829-
* if an error occurs.
1827+
* have been converted from the original input tuple after tuple routing.
1828+
* 'resultRelInfo' is the original result relation, before tuple routing.
18301829
*/
18311830
void
18321831
ExecConstraints(ResultRelInfo *resultRelInfo,
1833-
TupleTableSlot *slot, TupleTableSlot *orig_slot,
1834-
EState *estate)
1832+
TupleTableSlot *slot, EState *estate)
18351833
{
18361834
Relation rel = resultRelInfo->ri_RelationDesc;
18371835
TupleDesc tupdesc = RelationGetDescr(rel);
@@ -1854,23 +1852,37 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
18541852
{
18551853
char *val_desc;
18561854
Relation orig_rel = rel;
1857-
TupleDesc orig_tupdesc = tupdesc;
1855+
TupleDesc orig_tupdesc = RelationGetDescr(rel);
18581856

18591857
/*
1860-
* choose the correct relation to build val_desc from the
1861-
* tuple contained in orig_slot
1858+
* If the tuple has been routed, it's been converted to the
1859+
* partition's rowtype, which might differ from the root
1860+
* table's. We must convert it back to the root table's
1861+
* rowtype so that val_desc shown error message matches the
1862+
* input tuple.
18621863
*/
18631864
if (resultRelInfo->ri_PartitionRoot)
18641865
{
1866+
HeapTuple tuple = ExecFetchSlotTuple(slot);
1867+
TupleConversionMap *map;
1868+
18651869
rel = resultRelInfo->ri_PartitionRoot;
18661870
tupdesc = RelationGetDescr(rel);
1871+
/* a reverse map */
1872+
map = convert_tuples_by_name(orig_tupdesc, tupdesc,
1873+
gettext_noop("could not convert row type"));
1874+
if (map != NULL)
1875+
{
1876+
tuple = do_convert_tuple(tuple, map);
1877+
ExecStoreTuple(tuple, slot, InvalidBuffer, false);
1878+
}
18671879
}
18681880

18691881
insertedCols = GetInsertedColumns(resultRelInfo, estate);
18701882
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
18711883
modifiedCols = bms_union(insertedCols, updatedCols);
18721884
val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
1873-
orig_slot,
1885+
slot,
18741886
tupdesc,
18751887
modifiedCols,
18761888
64);
@@ -1897,15 +1909,27 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
18971909
/* See the comment above. */
18981910
if (resultRelInfo->ri_PartitionRoot)
18991911
{
1912+
HeapTuple tuple = ExecFetchSlotTuple(slot);
1913+
TupleDesc old_tupdesc = RelationGetDescr(rel);
1914+
TupleConversionMap *map;
1915+
19001916
rel = resultRelInfo->ri_PartitionRoot;
19011917
tupdesc = RelationGetDescr(rel);
1918+
/* a reverse map */
1919+
map = convert_tuples_by_name(old_tupdesc, tupdesc,
1920+
gettext_noop("could not convert row type"));
1921+
if (map != NULL)
1922+
{
1923+
tuple = do_convert_tuple(tuple, map);
1924+
ExecStoreTuple(tuple, slot, InvalidBuffer, false);
1925+
}
19021926
}
19031927

19041928
insertedCols = GetInsertedColumns(resultRelInfo, estate);
19051929
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
19061930
modifiedCols = bms_union(insertedCols, updatedCols);
19071931
val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
1908-
orig_slot,
1932+
slot,
19091933
tupdesc,
19101934
modifiedCols,
19111935
64);
@@ -1927,15 +1951,27 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
19271951
/* See the comment above. */
19281952
if (resultRelInfo->ri_PartitionRoot)
19291953
{
1954+
HeapTuple tuple = ExecFetchSlotTuple(slot);
1955+
TupleDesc old_tupdesc = RelationGetDescr(rel);
1956+
TupleConversionMap *map;
1957+
19301958
rel = resultRelInfo->ri_PartitionRoot;
19311959
tupdesc = RelationGetDescr(rel);
1960+
/* a reverse map */
1961+
map = convert_tuples_by_name(old_tupdesc, tupdesc,
1962+
gettext_noop("could not convert row type"));
1963+
if (map != NULL)
1964+
{
1965+
tuple = do_convert_tuple(tuple, map);
1966+
ExecStoreTuple(tuple, slot, InvalidBuffer, false);
1967+
}
19321968
}
19331969

19341970
insertedCols = GetInsertedColumns(resultRelInfo, estate);
19351971
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
19361972
modifiedCols = bms_union(insertedCols, updatedCols);
19371973
val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
1938-
orig_slot,
1974+
slot,
19391975
tupdesc,
19401976
modifiedCols,
19411977
64);

‎src/backend/executor/execReplication.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ ExecSimpleRelationInsert(EState *estate, TupleTableSlot *slot)
389389

390390
/* Check the constraints of the tuple */
391391
if (rel->rd_att->constr)
392-
ExecConstraints(resultRelInfo, slot, slot, estate);
392+
ExecConstraints(resultRelInfo, slot, estate);
393393

394394
/* Store the slot into tuple that we can inspect. */
395395
tuple = ExecMaterializeSlot(slot);
@@ -448,7 +448,7 @@ ExecSimpleRelationUpdate(EState *estate, EPQState *epqstate,
448448

449449
/* Check the constraints of the tuple */
450450
if (rel->rd_att->constr)
451-
ExecConstraints(resultRelInfo, slot, slot, estate);
451+
ExecConstraints(resultRelInfo, slot, estate);
452452

453453
/* Store the slot into tuple that we can write. */
454454
tuple = ExecMaterializeSlot(slot);

‎src/backend/executor/nodeModifyTable.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,7 @@ ExecInsert(ModifyTableState *mtstate,
263263
Relation resultRelationDesc;
264264
Oid newId;
265265
List *recheckIndexes = NIL;
266-
TupleTableSlot *oldslot = slot,
267-
*result = NULL;
266+
TupleTableSlot *result = NULL;
268267

269268
/*
270269
* get the heap tuple out of the tuple table slot, making sure we have a
@@ -435,7 +434,7 @@ ExecInsert(ModifyTableState *mtstate,
435434
* Check the constraints of the tuple
436435
*/
437436
if (resultRelationDesc->rd_att->constr || resultRelInfo->ri_PartitionCheck)
438-
ExecConstraints(resultRelInfo, slot, oldslot, estate);
437+
ExecConstraints(resultRelInfo, slot, estate);
439438

440439
if (onconflict != ONCONFLICT_NONE && resultRelInfo->ri_NumIndices > 0)
441440
{
@@ -993,7 +992,7 @@ lreplace:;
993992
* tuple-routing is performed here, hence the slot remains unchanged.
994993
*/
995994
if (resultRelationDesc->rd_att->constr || resultRelInfo->ri_PartitionCheck)
996-
ExecConstraints(resultRelInfo, slot, slot, estate);
995+
ExecConstraints(resultRelInfo, slot, estate);
997996

998997
/*
999998
* replace the heap tuple

‎src/include/executor/executor.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,7 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
186186
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
187187
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
188188
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
189-
TupleTableSlot *slot, TupleTableSlot *orig_slot,
190-
EState *estate);
189+
TupleTableSlot *slot, EState *estate);
191190
extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
192191
TupleTableSlot *slot, EState *estate);
193192
extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo);

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,11 +354,26 @@ ERROR: no partition of relation "mlparted1" found for row
354354
DETAIL: Partition key of the failing row contains ((b + 0)) = (5).
355355
truncate mlparted;
356356
alter table mlparted add constraint check_b check (b = 3);
357-
-- check that correct input row is shown when constraint check_b fails on mlparted11
358-
-- after "(1, 2)" is routed to it
357+
-- have a BR trigger modify the row such that the check_b is violated
358+
create function mlparted11_trig_fn()
359+
returns trigger AS
360+
$$
361+
begin
362+
NEW.b := 4;
363+
return NEW;
364+
end;
365+
$$
366+
language plpgsql;
367+
create trigger mlparted11_trig before insert ON mlparted11
368+
for each row execute procedure mlparted11_trig_fn();
369+
-- check that the correct row is shown when constraint check_b fails after
370+
-- "(1, 2)" is routed to mlparted11 (actually "(1, 4)" would be shown due
371+
-- to the BR trigger mlparted11_trig_fn)
359372
insert into mlparted values (1, 2);
360373
ERROR: new row for relation "mlparted11" violates check constraint "check_b"
361-
DETAIL: Failing row contains (1, 2).
374+
DETAIL: Failing row contains (1, 4).
375+
drop trigger mlparted11_trig on mlparted11;
376+
drop function mlparted11_trig_fn();
362377
-- check that inserting into an internal partition successfully results in
363378
-- checking its partition constraint before inserting into the leaf partition
364379
-- selected by tuple-routing

‎src/test/regress/sql/insert.sql

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,26 @@ insert into mlparted (a, b) values (1, 5);
217217

218218
truncate mlparted;
219219
alter table mlparted add constraint check_b check (b = 3);
220-
-- check that correct input row is shown when constraint check_b fails on mlparted11
221-
-- after "(1, 2)" is routed to it
220+
221+
-- have a BR trigger modify the row such that the check_b is violated
222+
create function mlparted11_trig_fn()
223+
returns trigger AS
224+
$$
225+
begin
226+
NEW.b := 4;
227+
return NEW;
228+
end;
229+
$$
230+
language plpgsql;
231+
create trigger mlparted11_trig before insert ON mlparted11
232+
for each row execute procedure mlparted11_trig_fn();
233+
234+
-- check that the correct row is shown when constraint check_b fails after
235+
-- "(1, 2)" is routed to mlparted11 (actually "(1, 4)" would be shown due
236+
-- to the BR trigger mlparted11_trig_fn)
222237
insert into mlparted values (1, 2);
238+
drop trigger mlparted11_trig on mlparted11;
239+
drop function mlparted11_trig_fn();
223240

224241
-- check that inserting into an internal partition successfully results in
225242
-- checking its partition constraint before inserting into the leaf partition

0 commit comments

Comments
 (0)