From 067096fa0c47191359911b1c2d441eae6ef99d9d Mon Sep 17 00:00:00 2001 From: soumyadeep2007 Date: Sun, 5 Jul 2020 10:23:30 -0700 Subject: [PATCH v1 1/1] Use table_tuple_fetch_row_version() to supply INSERT..RETURNING system cols 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 fetch the tuple being inserted anew using table_tuple_fetch_row_version() and pass it the slot we obtain from ExecGetReturningSlot(). This makes the assumption that such system columns would be present in the resulting slot. Then we pass this slot to ExecProcessReturning(). --- src/backend/executor/execExpr.c | 19 ++++++++++++++++++- src/backend/executor/nodeModifyTable.c | 20 ++++++++++++++++++++ src/include/nodes/execnodes.h | 2 ++ 3 files changed, 40 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..5af3eeb2e6 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -678,7 +678,27 @@ 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) + { + /* Fetch the inserted tuple to ensure that system columns are present. */ + TupleTableSlot *returningSlot = + ExecGetReturningSlot(estate, resultRelInfo); + Assert(!TTS_IS_VIRTUAL(returningSlot)); + if (!table_tuple_fetch_row_version(resultRelationDesc, + &slot->tts_tid, + SnapshotAny, + returningSlot)) + elog(ERROR, "failed to fetch inserted tuple for INSERT RETURNING"); + slot = returningSlot; + } 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