From 0c46b6fcfc2e1d28a6fc4967befb5437214c658a Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 15 Dec 2016 18:00:47 +0900
Subject: [PATCH 2/7] 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, also pass the original slot to ExecConstraints and use it to
build the val_desc to be shown in the messages.

Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
 src/backend/commands/copy.c            | 11 ++----
 src/backend/commands/tablecmds.c       |  1 +
 src/backend/executor/execMain.c        | 67 +++++++++++++++++++++++++++++-----
 src/backend/executor/nodeModifyTable.c | 15 +++-----
 src/include/executor/executor.h        |  4 +-
 src/include/nodes/execnodes.h          |  1 +
 src/test/regress/expected/insert.out   |  7 ++++
 src/test/regress/sql/insert.sql        |  6 +++
 8 files changed, 86 insertions(+), 26 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index e5a0f1bf80..afbfb9f6e8 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2430,6 +2430,7 @@ CopyFrom(CopyState cstate)
 					  cstate->rel,
 					  1,		/* dummy rangetable index */
 					  true,		/* do load partition check expression */
+					  NULL,
 					  0);
 
 	ExecOpenIndices(resultRelInfo, false);
@@ -2495,7 +2496,7 @@ CopyFrom(CopyState cstate)
 	for (;;)
 	{
 		TupleTableSlot *slot,
-					   *oldslot = NULL;
+					   *oldslot;
 		bool		skip_tuple;
 		Oid			loaded_oid = InvalidOid;
 
@@ -2537,6 +2538,7 @@ CopyFrom(CopyState cstate)
 		ExecStoreTuple(tuple, slot, InvalidBuffer, false);
 
 		/* Determine the partition to heap_insert the tuple into */
+		oldslot = slot;
 		if (cstate->partition_dispatch_info)
 		{
 			int			leaf_part_index;
@@ -2591,7 +2593,6 @@ CopyFrom(CopyState cstate)
 				 * point on.  Use a dedicated slot from this point on until
 				 * we're finished dealing with the partition.
 				 */
-				oldslot = slot;
 				slot = cstate->partition_tuple_slot;
 				Assert(slot != NULL);
 				ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
@@ -2628,7 +2629,7 @@ CopyFrom(CopyState cstate)
 				/* Check the constraints of the tuple */
 				if (cstate->rel->rd_att->constr ||
 					resultRelInfo->ri_PartitionCheck)
-					ExecConstraints(resultRelInfo, slot, estate);
+					ExecConstraints(resultRelInfo, slot, oldslot, estate);
 
 				if (useHeapMultiInsert)
 				{
@@ -2690,10 +2691,6 @@ CopyFrom(CopyState cstate)
 			{
 				resultRelInfo = saved_resultRelInfo;
 				estate->es_result_relation_info = resultRelInfo;
-
-				/* Switch back to the slot corresponding to the root table */
-				Assert(oldslot != NULL);
-				slot = oldslot;
 			}
 		}
 	}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a7ac85e7ab..c03edea18d 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 97c729d6b7..32c8f28beb 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);
@@ -1743,9 +1751,21 @@ ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
 	return ExecQual(resultRelInfo->ri_PartitionCheckExpr, econtext, true);
 }
 
+/*
+ * ExecConstraints - check constraints of the tuple in 'slot'
+ *
+ * This checks the traditional NOT NULL and check constraints, as well as
+ * the partition constraint, if any.
+ *
+ * Note: 'slot' contains the tuple to check the constraints of, which may
+ * have been converted from the original input tuple after tuple routing,
+ * while 'orig_slot' contains the original tuple to be shown in the message,
+ * if an error occurs.
+ */
 void
 ExecConstraints(ResultRelInfo *resultRelInfo,
-				TupleTableSlot *slot, EState *estate)
+				TupleTableSlot *slot, TupleTableSlot *orig_slot,
+				EState *estate)
 {
 	Relation	rel = resultRelInfo->ri_RelationDesc;
 	TupleDesc	tupdesc = RelationGetDescr(rel);
@@ -1767,12 +1787,24 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 				slot_attisnull(slot, attrChk))
 			{
 				char	   *val_desc;
+				Relation	orig_rel = rel;
+				TupleDesc	orig_tupdesc = tupdesc;
+
+				/*
+				 * choose the correct relation to build val_desc from the
+				 * tuple contained in orig_slot
+				 */
+				if (resultRelInfo->ri_PartitionRoot)
+				{
+					rel = resultRelInfo->ri_PartitionRoot;
+					tupdesc = RelationGetDescr(rel);
+				}
 
 				insertedCols = GetInsertedColumns(resultRelInfo, estate);
 				updatedCols = GetUpdatedColumns(resultRelInfo, estate);
 				modifiedCols = bms_union(insertedCols, updatedCols);
 				val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
-														 slot,
+														 orig_slot,
 														 tupdesc,
 														 modifiedCols,
 														 64);
@@ -1780,9 +1812,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,21 +1826,29 @@ 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)
+			{
+				rel = resultRelInfo->ri_PartitionRoot;
+				tupdesc = RelationGetDescr(rel);
+			}
 
 			insertedCols = GetInsertedColumns(resultRelInfo, estate);
 			updatedCols = GetUpdatedColumns(resultRelInfo, estate);
 			modifiedCols = bms_union(insertedCols, updatedCols);
 			val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
-													 slot,
+													 orig_slot,
 													 tupdesc,
 													 modifiedCols,
 													 64);
 			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,19 +1856,27 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 		!ExecPartitionCheck(resultRelInfo, slot, estate))
 	{
 		char	   *val_desc;
+		Relation	orig_rel = rel;
+
+		/* See the comment above. */
+		if (resultRelInfo->ri_PartitionRoot)
+		{
+			rel = resultRelInfo->ri_PartitionRoot;
+			tupdesc = RelationGetDescr(rel);
+		}
 
 		insertedCols = GetInsertedColumns(resultRelInfo, estate);
 		updatedCols = GetUpdatedColumns(resultRelInfo, estate);
 		modifiedCols = bms_union(insertedCols, updatedCols);
 		val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
-												 slot,
+												 orig_slot,
 												 tupdesc,
 												 modifiedCols,
 												 64);
 		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));
 	}
 }
@@ -3086,6 +3134,7 @@ ExecSetupPartitionTupleRouting(Relation rel,
 						  partrel,
 						  1,	 /* dummy */
 						  false,
+						  rel,
 						  0);
 
 		/*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index df21f66df8..825a15f42d 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -262,7 +262,7 @@ ExecInsert(ModifyTableState *mtstate,
 	Relation	resultRelationDesc;
 	Oid			newId;
 	List	   *recheckIndexes = NIL;
-	TupleTableSlot *oldslot = NULL;
+	TupleTableSlot *oldslot = slot;
 
 	/*
 	 * get the heap tuple out of the tuple table slot, making sure we have a
@@ -328,7 +328,6 @@ ExecInsert(ModifyTableState *mtstate,
 			 * point on, until we're finished dealing with the partition.
 			 * Use the dedicated slot for that.
 			 */
-			oldslot = slot;
 			slot = mtstate->mt_partition_tuple_slot;
 			Assert(slot != NULL);
 			ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
@@ -434,7 +433,7 @@ ExecInsert(ModifyTableState *mtstate,
 		 * Check the constraints of the tuple
 		 */
 		if (resultRelationDesc->rd_att->constr || resultRelInfo->ri_PartitionCheck)
-			ExecConstraints(resultRelInfo, slot, estate);
+			ExecConstraints(resultRelInfo, slot, oldslot, estate);
 
 		if (onconflict != ONCONFLICT_NONE && resultRelInfo->ri_NumIndices > 0)
 		{
@@ -579,10 +578,6 @@ ExecInsert(ModifyTableState *mtstate,
 	{
 		resultRelInfo = saved_resultRelInfo;
 		estate->es_result_relation_info = resultRelInfo;
-
-		/* Switch back to the slot corresponding to the root table */
-		Assert(oldslot != NULL);
-		slot = oldslot;
 	}
 
 	/*
@@ -994,10 +989,12 @@ lreplace:;
 								 resultRelInfo, slot, estate);
 
 		/*
-		 * Check the constraints of the tuple
+		 * Check the constraints of the tuple.  Note that we pass the same
+		 * slot for the orig_slot argument, because unlike ExecInsert(), no
+		 * tuple-routing is performed here, hence the slot remains unchanged.
 		 */
 		if (resultRelationDesc->rd_att->constr || resultRelInfo->ri_PartitionCheck)
-			ExecConstraints(resultRelInfo, slot, estate);
+			ExecConstraints(resultRelInfo, slot, slot, estate);
 
 		/*
 		 * replace the heap tuple
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index c217bd30cb..70ecf108a3 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -190,11 +190,13 @@ 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);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
-				TupleTableSlot *slot, EState *estate);
+				TupleTableSlot *slot, TupleTableSlot *orig_slot,
+				EState *estate);
 extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					 TupleTableSlot *slot, EState *estate);
 extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 3624660861..633c5cc107 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

