From baa48a89e571405f4cd802f065d0f82131599f53 Mon Sep 17 00:00:00 2001
From: soumyadeep2007 <soumyadeep2007@gmail.com>
Date: Sun, 5 Jul 2020 10:23:30 -0700
Subject: [PATCH v1 1/1] Explicitly supply system columns for INSERT..RETURNING
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If there are system columns other than ctid and tableOid in the
RETURNING clause for an INSERT, we must ensure that the slot we pass to
ExecProcessReturning() has those columns (ctid and tableOid are present
directly in the TupleTableSlot struct and can be satisifed from there:
refer: slot_getsysattr)).

This is in response to bug #16446 reported by Георгий Драк. In the bug
an INSERT..RETURNING statement fails with:

"ERROR: virtual tuple table slot does not have system
attributes"

This is caused due to the fact that ExecProcessReturning() was fed with
a virtual tuple table slot. Thus the resultant expression evaluation
and by extension, call to ExecEvalSysVar(), blows up with the
aforementioned error.

So, we fix this by:

1. Determining if there are system columns (other than tableOid and
ctid) referenced in ExecBuildProjectionInfo() and we store this info
inside the ProjectionInfo struct.

2. If there are such system columns in RETURNING, we explicitly create a
heap tuple table slot, fill in the system column values as we would do
during heap_prepare_insert() and then pass that slot to
ExecProcessReturning(). We use a heap tuple table slot as it is
guaranteed to support these attributes.
---
 src/backend/executor/execExpr.c        | 19 ++++++++++++++++++-
 src/backend/executor/nodeModifyTable.c | 23 +++++++++++++++++++++++
 src/include/nodes/execnodes.h          |  2 ++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 236413f62a..8cd966d227 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -360,10 +360,12 @@ ExecBuildProjectionInfo(List *targetList,
 	ExprState  *state;
 	ExprEvalStep scratch = {0};
 	ListCell   *lc;
+	List	   *tlist_vars;
 
 	projInfo->pi_exprContext = econtext;
 	/* We embed ExprState into ProjectionInfo instead of doing extra palloc */
 	projInfo->pi_state.tag = T_ExprState;
+	projInfo->has_non_slot_system_cols = false;
 	state = &projInfo->pi_state;
 	state->expr = (Expr *) targetList;
 	state->parent = parent;
@@ -455,7 +457,6 @@ ExecBuildProjectionInfo(List *targetList,
 			 */
 			ExecInitExprRec(tle->expr, state,
 							&state->resvalue, &state->resnull);
-
 			/*
 			 * Column might be referenced multiple times in upper nodes, so
 			 * force value to R/O - but only if it could be an expanded datum.
@@ -469,6 +470,22 @@ ExecBuildProjectionInfo(List *targetList,
 		}
 	}
 
+	/* Record system columns that are part of this projection */
+	tlist_vars = pull_var_clause((Node *) targetList,
+								 PVC_RECURSE_AGGREGATES |
+									 PVC_RECURSE_WINDOWFUNCS |
+									 PVC_INCLUDE_PLACEHOLDERS);
+	foreach(lc, tlist_vars)
+	{
+		Var *var = (Var *) lfirst(lc);
+		if (var->varattno < 0 && (var->varattno != TableOidAttributeNumber ||
+			var->varattno != SelfItemPointerAttributeNumber))
+		{
+			projInfo->has_non_slot_system_cols = true;
+			break;
+		}
+	}
+
 	scratch.opcode = EEOP_DONE;
 	ExprEvalPushStep(state, &scratch);
 
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 20a4c474cc..956bdf0afd 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -678,7 +678,30 @@ ExecInsert(ModifyTableState *mtstate,
 
 	/* Process RETURNING if present */
 	if (resultRelInfo->ri_projectReturning)
+	{
+		/*
+		 * If the RETURNING list contains system columns other than ctid and
+		 * tableOid, we should make sure that the system columns are available
+		 * in a slot that supports system columns.
+		 */
+		if (TTS_IS_VIRTUAL(slot) && resultRelInfo->ri_projectReturning->has_non_slot_system_cols)
+		{
+			/*
+			 * Explicitly supply the system columns that are not ctid and
+			 * tableOid. So, supply the system columns as we do when we insert.
+			 * See heap_prepare_insert().
+			 */
+			TupleTableSlot *returningSlot = ExecInitExtraTupleSlot(estate,
+																   RelationGetDescr(resultRelationDesc),
+																   &TTSOpsHeapTuple);
+			HeapTuple heapTuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+			HeapTupleHeaderSetXmin(heapTuple->t_data, GetCurrentTransactionId());
+			HeapTupleHeaderSetXmax(heapTuple->t_data, 0);
+			HeapTupleHeaderSetCmin(heapTuple->t_data, estate->es_output_cid);
+			slot = ExecStoreHeapTuple(heapTuple, returningSlot, true);
+		}
 		result = ExecProcessReturning(resultRelInfo, slot, planSlot);
+	}
 
 	return result;
 }
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index f5dfa32d55..f8047d2089 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -333,6 +333,8 @@ typedef struct ProjectionInfo
 	ExprState	pi_state;
 	/* expression context in which to evaluate expression */
 	ExprContext *pi_exprContext;
+	/* projection contains system columns other than ctid and tableOid */
+	bool		has_non_slot_system_cols;
 } ProjectionInfo;
 
 /* ----------------
-- 
2.24.1

