From 631d7490074cdaef8026db57a5f2772b8730f600 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 23 May 2015 00:49:27 +0200
Subject: [PATCH] Remove the new UPSERT command tag and use INSERT instead.
Previously, INSERT with ON CONFLICT DO UPDATE specified used a new
command tag -- UPSERT. It was introduced out of concern that INSERT as
a command tag would be a misrepresentation for ON CONFLICT DO UPDATE, as
some affected rows may actually have been updated.
Alvaro Herrera noticed that the implementation of that new command tag
was incomplete; in subsequent discussion we concluded that having it
doesn't provide benefits that are in line with the compatibility breaks
it requires.
Catversion bump due to the removal of PlannedStmt->isUpsert.
Author: Peter Geoghegan
Discussion: 20150520215816.GI5885@postgresql.org
---
doc/src/sgml/protocol.sgml | 13 +++----------
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/catalog/catversion.h | 2 +-
src/include/nodes/plannodes.h | 2 --
9 files changed, 15 insertions(+), 49 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index d985204566c..c7df697845e 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -3011,16 +3011,9 @@ CommandComplete (B)
<literal>INSERT <replaceable>oid</replaceable>
<replaceable>rows</replaceable></literal>, where
<replaceable>rows</replaceable> is the number of rows
- inserted. However, if and only if <literal>ON CONFLICT
- UPDATE</> is specified, then the tag is <literal>UPSERT
- <replaceable>oid</replaceable>
- <replaceable>rows</replaceable></literal>, where
- <replaceable>rows</replaceable> is the number of rows inserted
- <emphasis>or updated</emphasis>.
- <replaceable>oid</replaceable> is the object ID of the
- inserted row if <replaceable>rows</replaceable> is 1 and the
- target table has OIDs, and (for the <literal>UPSERT</literal>
- tag), the row was actually inserted rather than updated;
+ inserted. <replaceable>oid</replaceable> is the object ID
+ of the inserted row if <replaceable>rows</replaceable> is 1
+ and the target table has OIDs;
otherwise <replaceable>oid</replaceable> is 0.
</para>
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index 3c3315eab3d..7cd4577f1ea 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -497,20 +497,13 @@ INSERT INTO <replaceable class="PARAMETER">table_name</replaceable> [ AS <replac
<screen>
INSERT <replaceable>oid</replaceable> <replaceable class="parameter">count</replaceable>
</screen>
- However, in the event of an <literal>ON CONFLICT DO UPDATE</> clause
- (but <emphasis>not</emphasis> in the event of an <literal>ON
- CONFLICT DO NOTHING</> clause), the command tag reports the number of
- rows inserted or updated together, of the form
-<screen>
-UPSERT <replaceable>oid</replaceable> <replaceable class="parameter">count</replaceable>
-</screen>
- The <replaceable class="parameter">count</replaceable> is the number
- of rows inserted. If <replaceable class="parameter">count</replaceable>
- is exactly one, and the target table has OIDs, then
- <replaceable class="parameter">oid</replaceable> is the
- <acronym>OID</acronym>
- assigned to the inserted row (but not if there is only a single
- updated row). Otherwise <replaceable
+ The <replaceable class="parameter">count</replaceable> is the
+ number of rows inserted or updated. If <replaceable
+ class="parameter">count</replaceable> is exactly one, and the
+ target table has OIDs, then <replaceable
+ class="parameter">oid</replaceable> is the <acronym>OID</acronym>
+ assigned to the inserted row. The single row must have been
+ inserted rather than updated. Otherwise <replaceable
class="parameter">oid</replaceable> is zero.
</para>
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 2d9bf419bdb..cab93725e67 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 54464f8c656..4775acfcfc1 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 d3f5a140170..60340e39eda 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 bcffd85754c..9c14e8abdf8 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 f4155f78770..ff01368531a 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/catalog/catversion.h b/src/include/catalog/catversion.h
index 601258f2c2f..6b56fb5eb36 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201505191
+#define CATALOG_VERSION_NO 201505231
#endif
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index b70231919fe..61c84041407 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? */
--
GitLab