From c6df09f19533d2ee22882138eafa26c2fc82e261 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 15 Dec 2016 18:00:47 +0900 Subject: [PATCH 2/5] Make ExecConstraints() show 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 (ie, the table into which the row is inserted in a given query) rowtype and the partition's. To fix, convert the erring row back to root table's format before printing the error message showing the row. Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/commands/copy.c | 1 + src/backend/commands/tablecmds.c | 1 + src/backend/executor/execMain.c | 75 +++++++++++++++++++++++++++++++++--- src/include/executor/executor.h | 1 + src/include/nodes/execnodes.h | 1 + src/test/regress/expected/insert.out | 7 ++++ src/test/regress/sql/insert.sql | 6 +++ 7 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index aa25a23336..a20e22daaf 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2420,6 +2420,7 @@ CopyFrom(CopyState cstate) cstate->rel, 1, /* dummy rangetable index */ true, /* do load partition check expression */ + NULL, 0); ExecOpenIndices(resultRelInfo, false); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 115b98313e..338ff08ed4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1324,6 +1324,7 @@ ExecuteTruncate(TruncateStmt *stmt) rel, 0, /* dummy rangetable index */ false, + NULL, 0); resultRelInfo++; } diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index bca34a509c..55febd7bc1 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -828,6 +828,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) resultRelation, resultRelationIndex, true, + NULL, estate->es_instrument); resultRelInfo++; } @@ -1218,6 +1219,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, bool load_partition_check, + Relation partition_root, int instrument_options) { MemSet(resultRelInfo, 0, sizeof(ResultRelInfo)); @@ -1259,6 +1261,11 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo->ri_PartitionCheck = RelationGetPartitionQual(resultRelationDesc, true); + /* + * The following gets set to NULL unless we are initializing leaf + * partitions for tuple-routing. + */ + resultRelInfo->ri_PartitionRoot = partition_root; } /* @@ -1322,6 +1329,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) rel, 0, /* dummy rangetable index */ true, + NULL, estate->es_instrument); estate->es_trig_target_relations = lappend(estate->es_trig_target_relations, rInfo); @@ -1767,6 +1775,28 @@ 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); + TupleConversionMap *map; + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + /* a reverse map */ + map = convert_tuples_by_name(orig_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + tuple = do_convert_tuple(tuple, map); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1780,9 +1810,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))); } } } @@ -1794,6 +1824,23 @@ ExecConstraints(ResultRelInfo *resultRelInfo, if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL) { char *val_desc; + Relation orig_rel = rel; + TupleDesc orig_tupdesc = tupdesc; + + /* See the comment above. */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + TupleConversionMap *map; + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + /* a reverse map */ + map = convert_tuples_by_name(orig_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + tuple = do_convert_tuple(tuple, map); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1806,9 +1853,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))); } } @@ -1816,6 +1863,23 @@ ExecConstraints(ResultRelInfo *resultRelInfo, !ExecPartitionCheck(resultRelInfo, slot, estate)) { char *val_desc; + Relation orig_rel = rel; + TupleDesc orig_tupdesc = tupdesc; + + /* See the comment above. */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + TupleConversionMap *map; + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + /* a reverse map */ + map = convert_tuples_by_name(orig_tupdesc, tupdesc, + gettext_noop("could not convert row type")); + tuple = do_convert_tuple(tuple, map); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } insertedCols = GetInsertedColumns(resultRelInfo, estate); updatedCols = GetUpdatedColumns(resultRelInfo, estate); @@ -1828,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)); } } @@ -3074,6 +3138,7 @@ ExecSetupPartitionTupleRouting(Relation rel, partrel, 1, /* dummy */ false, + rel, 0); /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index b74fa5eb5d..f85b26b00a 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -190,6 +190,7 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, bool load_partition_check, + Relation partition_root, 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 9073a9a1bc..97b4083f14 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -349,6 +349,7 @@ typedef struct ResultRelInfo List *ri_onConflictSetWhere; List *ri_PartitionCheck; List *ri_PartitionCheckExpr; + Relation ri_PartitionRoot; } ResultRelInfo; /* ---------------- diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 49f667b119..b120954997 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -332,6 +332,13 @@ select tableoid::regclass, * from p; p11 | 1 | 2 (1 row) +truncate p; +alter table p add constraint check_b check (b = 3); +-- check that correct input row is shown when constraint check_b fails on p11 +-- after "(1, 2)" is routed to it +insert into p values (1, 2); +ERROR: new row for relation "p11" violates check constraint "check_b" +DETAIL: Failing row contains (1, 2). -- cleanup drop table p cascade; NOTICE: drop cascades to 2 other objects diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 08dc068de8..3d2fdb92c5 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -194,5 +194,11 @@ alter table p attach partition p1 for values from (1, 2) to (1, 10); insert into p values (1, 2); select tableoid::regclass, * from p; +truncate p; +alter table p add constraint check_b check (b = 3); +-- check that correct input row is shown when constraint check_b fails on p11 +-- after "(1, 2)" is routed to it +insert into p values (1, 2); + -- cleanup drop table p cascade; -- 2.11.0