Date: Thu, 6 Dec 2018 19:22:21 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: rewrite ExecPartitionCheckEmitError
Message-ID: <20181206222221.g5witbsklvqthjll@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ki3z2n5j73ow2eol"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: NeoMutt/20180716
Precedence: bulk


--ki3z2n5j73ow2eol
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Just on cleanliness grounds, I propose to rewrite the function in
$SUBJECT.  I came across this while reviewing some already-committed
patch for partition pruning, and it's been sitting in my laptop ever
since.

I think the current coding is too convoluted and hard to follow.  The
patch makes it much simpler (IMO).

-- 
Álvaro Herrera

--ki3z2n5j73ow2eol
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-rewrite-ExecPartitionCheckEmitError.patch"

From 8c8ac713ac168b2678925e58cdfc83694a8fa5b9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 14 Nov 2018 20:57:19 -0300
Subject: [PATCH] rewrite ExecPartitionCheckEmitError

---
 src/backend/executor/execMain.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d83d296d82..78c8ce4935 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1837,11 +1837,9 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
 							TupleTableSlot *slot,
 							EState *estate)
 {
-	Relation	rel = resultRelInfo->ri_RelationDesc;
-	Relation	orig_rel = rel;
-	TupleDesc	tupdesc = RelationGetDescr(rel);
-	char	   *val_desc;
-	Bitmapset  *modifiedCols;
+	Oid			root_relid;
+	TupleDesc	tupdesc;
+	char	   *valdsc;
 	Bitmapset  *insertedCols;
 	Bitmapset  *updatedCols;
 
@@ -1851,11 +1849,13 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
 	 */
 	if (resultRelInfo->ri_PartitionRoot)
 	{
-		TupleDesc	old_tupdesc = RelationGetDescr(rel);
+		TupleDesc	old_tupdesc;
 		AttrNumber *map;
 
-		rel = resultRelInfo->ri_PartitionRoot;
-		tupdesc = RelationGetDescr(rel);
+		root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
+
+		old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+		tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
 		/* a reverse map */
 		map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
 												gettext_noop("could not convert row type"));
@@ -1868,20 +1868,24 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
 			slot = execute_attr_map_slot(map, slot,
 										 MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
 	}
+	else
+	{
+		root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+		tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+	}
 
 	insertedCols = GetInsertedColumns(resultRelInfo, estate);
 	updatedCols = GetUpdatedColumns(resultRelInfo, estate);
-	modifiedCols = bms_union(insertedCols, updatedCols);
-	val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
-											 slot,
-											 tupdesc,
-											 modifiedCols,
-											 64);
+	valdsc = ExecBuildSlotValueDescription(root_relid,
+										   slot,
+										   tupdesc,
+										   bms_union(insertedCols, updatedCols),
+										   64);
 	ereport(ERROR,
 			(errcode(ERRCODE_CHECK_VIOLATION),
 			 errmsg("new row for relation \"%s\" violates partition constraint",
-					RelationGetRelationName(orig_rel)),
-			 val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
+					RelationGetRelationName(resultRelInfo->ri_RelationDesc)),
+			 valdsc ? errdetail("Failing row contains %s.", valdsc) : 0));
 }
 
 /*
-- 
2.11.0


--ki3z2n5j73ow2eol--