From c6df09f19533d2ee22882138eafa26c2fc82e261 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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

