From aa748c29286f916a286ff25ef25f0e2caa057058 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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.
---
 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 5627378a35..085a209882 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.
@@ -3098,10 +3155,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 8bcc876809..436d6937a9 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 be9a5e23cb..fadea14681 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

