From 3b50dcd242223c909bdedf20d9b7cb94ba2ff78e Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Fri, 22 May 2015 11:44:25 -0700 Subject: [PATCH] Remove "UPSERT" command tag Previously, INSERT with an ON CONFLICT DO UPDATE (but not an ON CONFLICT DO NOTHING) clause used a new command tag -- "UPSERT". This was mostly useful for psql, which reports the number of rows affected alongside the command tag. The original concern was that it would be a misrepresentation to allow this to happen with ON CONFLICT DO UPDATE, because some affected rows may actually have been updated. However, per discussion with Tom Lane, Andres Freund, and Alvaro Herrera, this seems unlikely to be worth it. There are compatibility problems with adding a new command tag, since it is for a command that is substantively the same as INSERT. Remove the recently added command tag, and update the INSERT documentation to reflect this. --- doc/src/sgml/ref/insert.sgml | 21 +++++++-------------- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/outfuncs.c | 1 - src/backend/optimizer/plan/planner.c | 2 -- src/backend/tcop/pquery.c | 17 +++-------------- src/bin/psql/common.c | 5 +---- src/include/nodes/plannodes.h | 2 -- 7 files changed, 11 insertions(+), 38 deletions(-) diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index 3c3315e..7cd4577 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -497,20 +497,13 @@ INSERT INTO table_name [ AS INSERT oid count - However, in the event of an ON CONFLICT DO UPDATE clause - (but not in the event of an ON - CONFLICT DO NOTHING clause), the command tag reports the number of - rows inserted or updated together, of the form - -UPSERT oid count - - The count is the number - of rows inserted. If count - is exactly one, and the target table has OIDs, then - oid is the - OID - assigned to the inserted row (but not if there is only a single - updated row). Otherwise count is the + number of rows inserted or updated. If count is exactly one, and the + target table has OIDs, then oid is the OID + assigned to the inserted row. The single row must have been + inserted rather than updated. Otherwise oid is zero. diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 2d9bf41..cab9372 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -81,7 +81,6 @@ _copyPlannedStmt(const PlannedStmt *from) COPY_SCALAR_FIELD(queryId); COPY_SCALAR_FIELD(hasReturning); COPY_SCALAR_FIELD(hasModifyingCTE); - COPY_SCALAR_FIELD(isUpsert); COPY_SCALAR_FIELD(canSetTag); COPY_SCALAR_FIELD(transientPlan); COPY_NODE_FIELD(planTree); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 54464f8..4775acf 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -243,7 +243,6 @@ _outPlannedStmt(StringInfo str, const PlannedStmt *node) WRITE_UINT_FIELD(queryId); WRITE_BOOL_FIELD(hasReturning); WRITE_BOOL_FIELD(hasModifyingCTE); - WRITE_BOOL_FIELD(isUpsert); WRITE_BOOL_FIELD(canSetTag); WRITE_BOOL_FIELD(transientPlan); WRITE_NODE_FIELD(planTree); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index d3f5a14..60340e3 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -261,8 +261,6 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) result->queryId = parse->queryId; result->hasReturning = (parse->returningList != NIL); result->hasModifyingCTE = parse->hasModifyingCTE; - result->isUpsert = - (parse->onConflict && parse->onConflict->action == ONCONFLICT_UPDATE); result->canSetTag = parse->canSetTag; result->transientPlan = glob->transientPlan; result->planTree = top_plan; diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index bcffd85..9c14e8a 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -202,14 +202,8 @@ ProcessQuery(PlannedStmt *plan, lastOid = queryDesc->estate->es_lastoid; else lastOid = InvalidOid; - if (plan->isUpsert) - snprintf(completionTag, COMPLETION_TAG_BUFSIZE, - "UPSERT %u %u", - lastOid, queryDesc->estate->es_processed); - else - snprintf(completionTag, COMPLETION_TAG_BUFSIZE, - "INSERT %u %u", - lastOid, queryDesc->estate->es_processed); + snprintf(completionTag, COMPLETION_TAG_BUFSIZE, + "INSERT %u %u", lastOid, queryDesc->estate->es_processed); break; case CMD_UPDATE: snprintf(completionTag, COMPLETION_TAG_BUFSIZE, @@ -1362,10 +1356,7 @@ 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 (unless the ON CONFLICT DO 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. + * one row was updated. See QueryRewrite(), step 3, for details. */ if (completionTag && completionTag[0] == '\0') { @@ -1375,8 +1366,6 @@ 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 f4155f7..ff01368 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -894,12 +894,9 @@ PrintQueryResults(PGresult *results) success = StoreQueryTuple(results); else success = PrintQueryTuples(results); - /* - * if it's INSERT/UPSERT/UPDATE/DELETE RETURNING, also print status - */ + /* if it's INSERT/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); diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index b702319..61c8404 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -45,8 +45,6 @@ typedef struct PlannedStmt bool hasModifyingCTE; /* has insert|update|delete in WITH? */ - bool isUpsert; /* is it insert ... ON CONFLICT UPDATE? */ - bool canSetTag; /* do I set the command result tag? */ bool transientPlan; /* redo plan when TransactionXmin changes? */ -- 1.9.1