From ef83ef4cc90a43acf22c097480f71b2b3c1c30f1 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 15 Dec 2016 18:00:47 +0900 Subject: [PATCH 6/7] Make ExecConstraints() emit the correct row in error msgs After a tuple is routed to a partition, it has been converted from the root table's rowtype to the partition's. If such a tuple causes an error in ExecConstraints(), the row shown in error messages might not match the input row due to possible differences between the root table's rowtype and the partition's. To convert back to the correct row format, keep root table relation descriptor and a reverse tuple conversion map in the ResultRelInfo's of leaf partitions. Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/commands/copy.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/backend/executor/execMain.c | 83 ++++++++++++++++++++++++++++++++---- src/include/executor/executor.h | 2 + src/include/nodes/execnodes.h | 2 + src/test/regress/expected/insert.out | 15 ++++++- src/test/regress/sql/insert.sql | 11 +++++ 7 files changed, 106 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index bec8c73903..9cd84e80c7 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2424,7 +2424,7 @@ CopyFrom(CopyState cstate) InitResultRelInfo(resultRelInfo, cstate->rel, 1, /* dummy rangetable index */ - partcheck, + partcheck, NULL, NULL, 0); ExecOpenIndices(resultRelInfo, false); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4bd4ec4e8c..67ff1715ea 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1323,7 +1323,7 @@ ExecuteTruncate(TruncateStmt *stmt) InitResultRelInfo(resultRelInfo, rel, 0, /* dummy rangetable index */ - NIL, + NIL, NULL, NULL, 0); resultRelInfo++; } diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 1b19bc38ef..b56fd8ca6f 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -833,7 +833,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) InitResultRelInfo(resultRelInfo, resultRelation, resultRelationIndex, - partcheck, + partcheck, NULL, NULL, estate->es_instrument); resultRelInfo++; } @@ -1224,6 +1224,8 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, List *partition_check, + Relation partition_root, + TupleConversionMap *partition_reverse_map, int instrument_options) { MemSet(resultRelInfo, 0, sizeof(ResultRelInfo)); @@ -1262,6 +1264,12 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo->ri_junkFilter = NULL; resultRelInfo->ri_projectReturning = NULL; resultRelInfo->ri_PartitionCheck = partition_check; + /* + * Following fields are only looked at in some tuple-routing cases. + * In other case, they are set to NULL. + */ + resultRelInfo->ri_PartitionRoot = partition_root; + resultRelInfo->ri_PartitionReverseMap = partition_reverse_map; } /* @@ -1329,7 +1337,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) InitResultRelInfo(rInfo, rel, 0, /* dummy rangetable index */ - partcheck, + partcheck, NULL, NULL, estate->es_instrument); estate->es_trig_target_relations = lappend(estate->es_trig_target_relations, rInfo); @@ -1775,6 +1783,26 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot_attisnull(slot, attrChk)) { char *val_desc; + Relation orig_rel = rel; + TupleDesc orig_tupdesc = tupdesc; + + /* + * In case where the tuple is routed, it's been converted + * to the partition's rowtype, which might differ from the + * root table's. We must convert it back to the root table's + * type so that it's shown correctly in the error message. + */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + Assert(resultRelInfo->ri_PartitionReverseMap != NULL); + tuple = do_convert_tuple(tuple, + resultRelInfo->ri_PartitionReverseMap); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1788,9 +1816,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo, ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("null value in column \"%s\" violates not-null constraint", - NameStr(tupdesc->attrs[attrChk - 1]->attname)), + NameStr(orig_tupdesc->attrs[attrChk - 1]->attname)), val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, - errtablecol(rel, attrChk))); + errtablecol(orig_rel, attrChk))); } } } @@ -1802,6 +1830,20 @@ ExecConstraints(ResultRelInfo *resultRelInfo, if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL) { char *val_desc; + Relation orig_rel = rel; + + /* See the comment above. */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + Assert(resultRelInfo->ri_PartitionReverseMap != NULL); + tuple = do_convert_tuple(tuple, + resultRelInfo->ri_PartitionReverseMap); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1814,9 +1856,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo, ereport(ERROR, (errcode(ERRCODE_CHECK_VIOLATION), errmsg("new row for relation \"%s\" violates check constraint \"%s\"", - RelationGetRelationName(rel), failed), + RelationGetRelationName(orig_rel), failed), val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, - errtableconstraint(rel, failed))); + errtableconstraint(orig_rel, failed))); } } @@ -1824,6 +1866,20 @@ ExecConstraints(ResultRelInfo *resultRelInfo, !ExecPartitionCheck(resultRelInfo, slot, estate)) { char *val_desc; + Relation orig_rel = rel; + + /* See the comment above. */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + Assert(resultRelInfo->ri_PartitionReverseMap != NULL); + tuple = do_convert_tuple(tuple, + resultRelInfo->ri_PartitionReverseMap); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1836,7 +1892,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo, ereport(ERROR, (errcode(ERRCODE_CHECK_VIOLATION), errmsg("new row for relation \"%s\" violates partition constraint", - RelationGetRelationName(rel)), + RelationGetRelationName(orig_rel)), val_desc ? errdetail("Failing row contains %s.", val_desc) : 0)); } } @@ -3068,6 +3124,7 @@ ExecSetupPartitionTupleRouting(Relation rel, Relation partrel; TupleDesc part_tupdesc; List *my_check = NIL; + TupleConversionMap *reverse_map; /* * We locked all the partitions above including the leaf partitions. @@ -3097,10 +3154,20 @@ ExecSetupPartitionTupleRouting(Relation rel, if (partcheck) my_check = map_partition_varattnos(partcheck, partrel, rel); + /* + * We must save a reverse tuple conversion map as well, to show the + * correct input tuple in the error message shown by ExecConstraints() + * in case of routed tuples. Remember that at the point of failure, + * the tuple has been converted to the partition's type which might + * not match the input tuple. + */ + reverse_map = convert_tuples_by_name(part_tupdesc, tupDesc, + gettext_noop("could not convert row type")); + InitResultRelInfo(leaf_part_rri, partrel, 1, /* dummy */ - my_check, + my_check, rel, reverse_map, 0); /* Open partition indices */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index c8e42ae2eb..3f91d5734e 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -189,6 +189,8 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, List *partition_check, + Relation partition_root, + TupleConversionMap *partition_reverse_map, int instrument_options); extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid); extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index f49702b122..6d06af949a 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -349,6 +349,8 @@ typedef struct ResultRelInfo List *ri_onConflictSetWhere; List *ri_PartitionCheck; List *ri_PartitionCheckExpr; + Relation ri_PartitionRoot; + TupleConversionMap *ri_PartitionReverseMap; } ResultRelInfo; /* ---------------- diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 95a7c4da7a..328f776dd7 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -289,6 +289,18 @@ select tableoid::regclass, * from list_parted; insert into part_ee_ff values ('gg', 1); ERROR: new row for relation "part_ee_ff1" violates partition constraint DETAIL: Failing row contains (gg, 1). +-- check that correct row is shown in the error message if constraint fails +-- after a tuple is routed to a partition with different rowtype from the +-- root table +create table part_ee_ff3 (like part_ee_ff); +alter table part_ee_ff3 drop a; +alter table part_ee_ff3 add a text; -- (a's attnum is now 3) +alter table part_ee_ff attach partition part_ee_ff3 for values from (20) to (30); +truncate part_ee_ff; +alter table part_ee_ff add constraint check_b_25 check (b = 25); +insert into list_parted values ('ee', 20); +ERROR: new row for relation "part_ee_ff3" violates check constraint "check_b_25" +DETAIL: Failing row contains (ee, 20). -- cleanup drop table range_parted cascade; NOTICE: drop cascades to 4 other objects @@ -297,10 +309,11 @@ drop cascades to table part2 drop cascades to table part3 drop cascades to table part4 drop table list_parted cascade; -NOTICE: drop cascades to 6 other objects +NOTICE: drop cascades to 7 other objects DETAIL: drop cascades to table part_aa_bb drop cascades to table part_cc_dd drop cascades to table part_null drop cascades to table part_ee_ff drop cascades to table part_ee_ff1 drop cascades to table part_ee_ff2 +drop cascades to table part_ee_ff3 diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 77577682ac..ebe371e884 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -170,6 +170,17 @@ select tableoid::regclass, * from list_parted; -- fail due to partition constraint failure insert into part_ee_ff values ('gg', 1); +-- check that correct row is shown in the error message if constraint fails +-- after a tuple is routed to a partition with different rowtype from the +-- root table +create table part_ee_ff3 (like part_ee_ff); +alter table part_ee_ff3 drop a; +alter table part_ee_ff3 add a text; -- (a's attnum is now 3) +alter table part_ee_ff attach partition part_ee_ff3 for values from (20) to (30); +truncate part_ee_ff; +alter table part_ee_ff add constraint check_b_25 check (b = 25); +insert into list_parted values ('ee', 20); + -- cleanup drop table range_parted cascade; drop table list_parted cascade; -- 2.11.0