From c40c1dfb555a65d25dcab12409ebef9b903f6e78 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Fri, 21 Nov 2014 16:59:54 -0800 Subject: [PATCH 4/8] Project updates from ON CONFLICT UPDATE RETURNING This establishes that an INSERT with an ON CONFLICT UPDATE clause processes all slots that are ultimately affected, regardless of whether or not the alternative ON CONFLICT UPDATE path was taken. However, if an ON CONFLICT UPDATE's WHERE clause is not satisfied in respect of some slot/tuple, the post-update tuple is not projected (although the row is still locked, just as before). Also, for ON CONFLICT UPDATE variant INSERTs (but not ON CONFLICT IGNORE variant INSERTs), the number of rows affected in total is reported by the command tag using the new "UPSERT" command identifier, which otherwise matches the format of the existing "INSERT" command tag. There is no precedent for a top level command that uses a different command tag identifier according to whether or not some clause was used, but doing so seems appropriate, since client programs are expected to have an interest in whether or not some number of rows projected by RETURNING may have been updated, and in any case indicating that the rows were affected by an "INSERT" when they may not have been inserted is simply misleading. However, there is still no principled method for client programs to distinguish between INSERT ... ON CONFLICT UPDATE projected tuples generated by being inserted or by being updated. This is thought not to matter, since the use of INSERT with ON CONFLICT UPDATE indicates that either outcome is equivalent. --- src/backend/executor/nodeModifyTable.c | 30 ++++++++++++++++++++++-------- src/backend/tcop/pquery.c | 16 +++++++++++++--- src/bin/psql/common.c | 5 ++++- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 05c78c9..1603c45 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -59,7 +59,9 @@ static bool ExecLockUpdateTuple(ResultRelInfo *resultRelInfo, TupleTableSlot *planSlot, TupleTableSlot *insertSlot, ModifyTableState *onConflict, - EState *estate); + EState *estate, + bool canSetTag, + TupleTableSlot **returning); /* * Verify that the tuples to be produced by INSERT or UPDATE match the @@ -413,6 +415,8 @@ vlock: if (conflict) { + TupleTableSlot *returning = NULL; + /* * Lock and consider updating in the SPEC_INSERT case. For the * SPEC_IGNORE case, it's still necessary to verify that the tuple @@ -423,12 +427,20 @@ vlock: planSlot, slot, onConflict, - estate)) + estate, + canSetTag, + &returning)) goto vlock; else if (spec == SPEC_IGNORE) ExecCheckHeapTupleVisible(estate, resultRelInfo, &conflictTid); - return NULL; + /* + * RETURNING may have been processed already -- the target + * ResultRelInfo might have made representation within ExecUpdate() + * that this is required. Inserted and updated tuples are + * projected indifferently for ON CONFLICT UPDATE with RETURNING. + */ + return returning; } } @@ -967,7 +979,9 @@ ExecLockUpdateTuple(ResultRelInfo *resultRelInfo, TupleTableSlot *planSlot, TupleTableSlot *insertSlot, ModifyTableState *onConflict, - EState *estate) + EState *estate, + bool canSetTag, + TupleTableSlot **returning) { Relation relation = resultRelInfo->ri_RelationDesc; HeapTupleData tuple; @@ -1135,9 +1149,9 @@ ExecLockUpdateTuple(ResultRelInfo *resultRelInfo, slot = EvalPlanQualNext(&onConflict->mt_epqstate); if (!TupIsNull(slot)) - ExecUpdate(&tuple.t_data->t_ctid, NULL, slot, planSlot, - &onConflict->mt_epqstate, onConflict->ps.state, - false); + *returning = ExecUpdate(&tuple.t_data->t_ctid, NULL, slot, + planSlot, &onConflict->mt_epqstate, + onConflict->ps.state, canSetTag); ReleaseBuffer(buffer); @@ -1149,7 +1163,7 @@ ExecLockUpdateTuple(ResultRelInfo *resultRelInfo, /* must provide our own instrumentation support */ if (onConflict->ps.instrument) - InstrStopNode(onConflict->ps.instrument, 0); + InstrStopNode(onConflict->ps.instrument, *returning ? 1:0); return true; case HeapTupleUpdated: diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 9c14e8a..41c4191 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -189,7 +189,8 @@ ProcessQuery(PlannedStmt *plan, */ if (completionTag) { - Oid lastOid; + Oid lastOid; + ModifyTableState *pstate; switch (queryDesc->operation) { @@ -198,12 +199,16 @@ ProcessQuery(PlannedStmt *plan, "SELECT %u", queryDesc->estate->es_processed); break; case CMD_INSERT: + pstate = (((ModifyTableState *) queryDesc->planstate)); + Assert(IsA(pstate, ModifyTableState)); + if (queryDesc->estate->es_processed == 1) lastOid = queryDesc->estate->es_lastoid; else lastOid = InvalidOid; snprintf(completionTag, COMPLETION_TAG_BUFSIZE, - "INSERT %u %u", lastOid, queryDesc->estate->es_processed); + "%s %u %u", pstate->spec == SPEC_INSERT? "UPSERT":"INSERT", + lastOid, queryDesc->estate->es_processed); break; case CMD_UPDATE: snprintf(completionTag, COMPLETION_TAG_BUFSIZE, @@ -1356,7 +1361,10 @@ PortalRunMulti(Portal portal, bool isTopLevel, * 0" here because technically there is no query of the matching tag type, * and printing a non-zero count for a different query type seems wrong, * e.g. an INSERT that does an UPDATE instead should not print "0 1" if - * one row was updated. See QueryRewrite(), step 3, for details. + * one row was updated (unless the ON CONFLICT UPDATE, or "UPSERT" variant + * of INSERT was used to update the row, where it's logically a direct + * effect of the top level command). See QueryRewrite(), step 3, for + * details. */ if (completionTag && completionTag[0] == '\0') { @@ -1366,6 +1374,8 @@ PortalRunMulti(Portal portal, bool isTopLevel, sprintf(completionTag, "SELECT 0 0"); else if (strcmp(completionTag, "INSERT") == 0) strcpy(completionTag, "INSERT 0 0"); + else if (strcmp(completionTag, "UPSERT") == 0) + strcpy(completionTag, "UPSERT 0 0"); else if (strcmp(completionTag, "UPDATE") == 0) strcpy(completionTag, "UPDATE 0"); else if (strcmp(completionTag, "DELETE") == 0) diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 275bdcc..9302e41 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -894,9 +894,12 @@ PrintQueryResults(PGresult *results) success = StoreQueryTuple(results); else success = PrintQueryTuples(results); - /* if it's INSERT/UPDATE/DELETE RETURNING, also print status */ + /* + * if it's INSERT/UPSERT/UPDATE/DELETE RETURNING, also print status + */ cmdstatus = PQcmdStatus(results); if (strncmp(cmdstatus, "INSERT", 6) == 0 || + strncmp(cmdstatus, "UPSERT", 6) == 0 || strncmp(cmdstatus, "UPDATE", 6) == 0 || strncmp(cmdstatus, "DELETE", 6) == 0) PrintQueryStatus(results); -- 1.9.1