> > On 3/4/25 10:24 AM, Andreas Karlsson wrote:
> > Rebased the patch to add support for OLD.* and NEW.*.
>
I took a closer look at this, and I have a number of comments:
* The changes for RLS look correct. However, in
get_row_security_policies(), it's not necessary to add
WCO_RLS_UPDATE_CHECK checks from UPDATE policies when doing ON
CONFLICT DO SELECT because it's not going to do an UPDATE. Also, some
code duplication can be avoided, because basically the plain DO SELECT
case is the same as the other cases, but with some checks not
required. So it's much simpler to leave the code structure as it was,
and just disable the checks that aren't needed based on the
permissions required by the command being executed, which IMO makes
the code easier to follow.
* In ExecOnConflictSelect(), the comment block for RLS checking seems
to have been copied verbatim from ExecOnConflictUpdate(), but it needs
updating, because the two cases are different.
* As I mentioned before, I think that when returning OLD/NEW values,
ON CONFLICT DO SELECT should behave the same as ON CONFLICT DO UPDATE
would do if it didn't change anything.
* Looking at the WHERE clause, I think it's a mistake to not allow
excluded values in it. That makes the WHERE clause of ON CONFLICT DO
SELECT inconsistent with the WHERE clause of ON CONFLICT DO UPDATE.
And that inconsistency might make it tricky to add support for
excluded later, since it requires the use of qualified column names.
It seems to me that it might be very useful to be able to refer to
excluded values -- e.g., to return just the rows that where different
from what it tried to insert -- and supporting it only requires minor
tweaks to transformOnConflictClause(), set_plan_refs(), and
ExecOnConflictSelect().
* ruleutils.c needs support for deparsing ON CONFLICT DO SELECT.
* When inserting into a table with a rule, if the rule action is
INSERT ... ON CONFLICT DO SELECT ... RETURNING, then the triggering
query must also have a RETURNING clause. The parser check for a
RETURNING clause doesn't catch that, so there needs to also be a check
in the rewriter.
Attached is an update, with fixes for those issues, plus a bit of
miscellaneous tidying up (as a separate patch for ease of review).
There's at least one more code issue that I didn't have time to look at:
create table foo (a int primary key, b text) partition by list (a);
create table foo_p1 partition of foo for values in (1);
create table foo_p2 partition of foo for values in (2);
insert into foo values (1, 'xxx')
on conflict (a) do select for update returning *;
insert into foo values (1, 'xxx')
on conflict (a) do select for update returning *;
server closed the connection unexpectedly
It looks to me like ExecInitPartitionInfo() needs updating to
initialise the WHERE clause for ON CONFLICT DO SELECT.
I think there is still a fair bit more to do to get this into a
committable state. The docs in particular need work. For example, on
the INSERT page:
* The INSERT synopsis fails to mention that DO SELECT supports WHERE.
* The paragraph about privileges needs updating for DO SELECT.
* The final paragraph under "output_expression" should mention DO SELECT.
* The "ON CONFLICT clause" section doesn't mention DO SELECT at all.
* The "Outputs" section should mention select.
* The "Examples" section should have at least one example of DO SELECT.
The penultimate paragraph of section 6.4, "Returning Data from
Modified Rows" also needs updating. There may be more places. I'd
suggest a bit of grepping in the docs (and probably also in the code)
for other places that need updating.
It also feels like this needs more regression tests, plus some new
isolation test cases.
Regards,
Dean
From ea50933617ec2fd42ffe1927cbfc7765b1d8ab3c Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <andr...@proxel.se>
Date: Mon, 18 Nov 2024 00:29:15 +0100
Subject: [PATCH v7 1/2] Add support for ON CONFLICT DO SELECT [ FOR ... ]
Adds support for DO SELECT action for ON CONFLICT clause where we
select the tuples and optionally lock them. If the tuples are locked
with check for conflicts, otherwise not.
---
doc/src/sgml/ref/insert.sgml | 17 +-
src/backend/commands/explain.c | 33 ++-
src/backend/executor/nodeModifyTable.c | 278 +++++++++++++++---
src/backend/optimizer/plan/createplan.c | 2 +
src/backend/parser/analyze.c | 26 +-
src/backend/parser/gram.y | 20 +-
src/backend/parser/parse_clause.c | 7 +
src/backend/rewrite/rowsecurity.c | 42 ++-
src/include/nodes/execnodes.h | 2 +
src/include/nodes/lockoptions.h | 3 +-
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h | 4 +-
src/include/nodes/plannodes.h | 2 +
src/include/nodes/primnodes.h | 9 +-
src/test/regress/expected/insert_conflict.out | 151 ++++++++--
src/test/regress/sql/insert_conflict.sql | 79 +++--
16 files changed, 571 insertions(+), 105 deletions(-)
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index 3f139917790..6f4de8ab090 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -37,6 +37,7 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac
<phrase>and <replaceable class="parameter">conflict_action</replaceable> is one of:</phrase>
DO NOTHING
+ DO SELECT [ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } ]
DO UPDATE SET { <replaceable class="parameter">column_name</replaceable> = { <replaceable class="parameter">expression</replaceable> | DEFAULT } |
( <replaceable class="parameter">column_name</replaceable> [, ...] ) = [ ROW ] ( { <replaceable class="parameter">expression</replaceable> | DEFAULT } [, ...] ) |
( <replaceable class="parameter">column_name</replaceable> [, ...] ) = ( <replaceable class="parameter">sub-SELECT</replaceable> )
@@ -88,18 +89,24 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac
<para>
The optional <literal>RETURNING</literal> clause causes <command>INSERT</command>
- to compute and return value(s) based on each row actually inserted
- (or updated, if an <literal>ON CONFLICT DO UPDATE</literal> clause was
- used). This is primarily useful for obtaining values that were
+ to compute and return value(s) based on each row actually inserted.
+ If an <literal>ON CONFLICT DO UPDATE</literal> clause was used,
+ <literal>RETURNING</literal> also returns tuples which were updated, and
+ in the presence of an <literal>ON CONFLICT DO SELECT</literal> clause all
+ input rows are returned. With a traditional <command>INSERT</command>,
+ the <literal>RETURNING</literal> clause is primarily useful for obtaining
+ values that were
supplied by defaults, such as a serial sequence number. However,
any expression using the table's columns is allowed. The syntax of
the <literal>RETURNING</literal> list is identical to that of the output
- list of <command>SELECT</command>. Only rows that were successfully
+ list of <command>SELECT</command>. If an <literal>ON CONFLICT DO SELECT</literal>
+ clause is not present, only rows that were successfully
inserted or updated will be returned. For example, if a row was
locked but not updated because an <literal>ON CONFLICT DO UPDATE
... WHERE</literal> clause <replaceable
class="parameter">condition</replaceable> was not satisfied, the
- row will not be returned.
+ row will not be returned. <literal>ON CONFLICT DO SELECT</literal>
+ works similarly, except no update takes place.
</para>
<para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ef8aa489af8..ecb0bbeaa9c 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4651,10 +4651,35 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
if (node->onConflictAction != ONCONFLICT_NONE)
{
- ExplainPropertyText("Conflict Resolution",
- node->onConflictAction == ONCONFLICT_NOTHING ?
- "NOTHING" : "UPDATE",
- es);
+ const char *resolution;
+
+ if (node->onConflictAction == ONCONFLICT_NOTHING)
+ resolution = "NOTHING";
+ else if (node->onConflictAction == ONCONFLICT_UPDATE)
+ resolution = "UPDATE";
+ else
+ {
+ switch (node->onConflictLockingStrength)
+ {
+ case LCS_NONE:
+ resolution = "SELECT";
+ break;
+ case LCS_FORKEYSHARE:
+ resolution = "SELECT FOR KEY SHARE";
+ break;
+ case LCS_FORSHARE:
+ resolution = "SELECT FOR SHARE";
+ break;
+ case LCS_FORNOKEYUPDATE:
+ resolution = "SELECT FOR NO KEY UPDATE";
+ break;
+ default: /* LCS_FORUPDATE */
+ resolution = "SELECT FOR UPDATE";
+ break;
+ }
+ }
+
+ ExplainPropertyText("Conflict Resolution", resolution, es);
/*
* Don't display arbiter indexes at all when DO NOTHING variant
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 309e27f8b5f..250685e2eda 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -145,12 +145,23 @@ static void ExecCrossPartitionUpdateForeignKey(ModifyTableContext *context,
ItemPointer tupleid,
TupleTableSlot *oldslot,
TupleTableSlot *newslot);
+static bool ExecOnConflictLockRow(ModifyTableContext *context,
+ TupleTableSlot *existing,
+ ItemPointer conflictTid,
+ Relation relation,
+ LockTupleMode lockmode,
+ bool isUpdate);
static bool ExecOnConflictUpdate(ModifyTableContext *context,
ResultRelInfo *resultRelInfo,
ItemPointer conflictTid,
TupleTableSlot *excludedSlot,
bool canSetTag,
TupleTableSlot **returning);
+static bool ExecOnConflictSelect(ModifyTableContext *context,
+ ResultRelInfo *resultRelInfo,
+ ItemPointer conflictTid,
+ bool canSetTag,
+ TupleTableSlot **returning);
static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
EState *estate,
PartitionTupleRouting *proute,
@@ -1138,6 +1149,26 @@ ExecInsert(ModifyTableContext *context,
else
goto vlock;
}
+ else if (onconflict == ONCONFLICT_SELECT)
+ {
+ /*
+ * In case of ON CONFLICT DO SELECT, optionally lock the
+ * conflicting tuple, fetch it and project RETURNING on
+ * it. Be prepared to retry if fetching fails because of a
+ * concurrent UPDATE/DELETE to the conflict tuple.
+ */
+ TupleTableSlot *returning = NULL;
+
+ if (ExecOnConflictSelect(context, resultRelInfo,
+ &conflictTid, canSetTag,
+ &returning))
+ {
+ InstrCountTuples2(&mtstate->ps, 1);
+ return returning;
+ }
+ else
+ goto vlock;
+ }
else
{
/*
@@ -2676,52 +2707,26 @@ redo_act:
}
/*
- * ExecOnConflictUpdate --- execute UPDATE of INSERT ON CONFLICT DO UPDATE
- *
- * Try to lock tuple for update as part of speculative insertion. If
- * a qual originating from ON CONFLICT DO UPDATE is satisfied, update
- * (but still lock row, even though it may not satisfy estate's
- * snapshot).
- *
- * Returns true if we're done (with or without an update), or false if
- * the caller must retry the INSERT from scratch.
+ * ExecOnConflictLockRow --- lock the row for ON CONFLICT DO UPDATE/SELECT
*/
static bool
-ExecOnConflictUpdate(ModifyTableContext *context,
- ResultRelInfo *resultRelInfo,
- ItemPointer conflictTid,
- TupleTableSlot *excludedSlot,
- bool canSetTag,
- TupleTableSlot **returning)
+ExecOnConflictLockRow(ModifyTableContext *context,
+ TupleTableSlot *existing,
+ ItemPointer conflictTid,
+ Relation relation,
+ LockTupleMode lockmode,
+ bool isUpdate)
{
- ModifyTableState *mtstate = context->mtstate;
- ExprContext *econtext = mtstate->ps.ps_ExprContext;
- Relation relation = resultRelInfo->ri_RelationDesc;
- ExprState *onConflictSetWhere = resultRelInfo->ri_onConflict->oc_WhereClause;
- TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing;
TM_FailureData tmfd;
- LockTupleMode lockmode;
TM_Result test;
Datum xminDatum;
TransactionId xmin;
bool isnull;
/*
- * Parse analysis should have blocked ON CONFLICT for all system
- * relations, which includes these. There's no fundamental obstacle to
- * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other
- * ExecUpdate() caller.
- */
- Assert(!resultRelInfo->ri_needLockTagTuple);
-
- /* Determine lock mode to use */
- lockmode = ExecUpdateLockMode(context->estate, resultRelInfo);
-
- /*
- * Lock tuple for update. Don't follow updates when tuple cannot be
- * locked without doing so. A row locking conflict here means our
- * previous conclusion that the tuple is conclusively committed is not
- * true anymore.
+ * Don't follow updates when tuple cannot be locked without doing so. A
+ * row locking conflict here means our previous conclusion that the tuple
+ * is conclusively committed is not true anymore.
*/
test = table_tuple_lock(relation, conflictTid,
context->estate->es_snapshot,
@@ -2763,7 +2768,7 @@ ExecOnConflictUpdate(ModifyTableContext *context,
(errcode(ERRCODE_CARDINALITY_VIOLATION),
/* translator: %s is a SQL command name */
errmsg("%s command cannot affect row a second time",
- "ON CONFLICT DO UPDATE"),
+ isUpdate ? "ON CONFLICT DO UPDATE" : "ON CONFLICT DO SELECT"),
errhint("Ensure that no rows proposed for insertion within the same command have duplicate constrained values.")));
/* This shouldn't happen */
@@ -2820,6 +2825,50 @@ ExecOnConflictUpdate(ModifyTableContext *context,
}
/* Success, the tuple is locked. */
+ return true;
+}
+
+/*
+ * ExecOnConflictUpdate --- execute UPDATE of INSERT ON CONFLICT DO UPDATE
+ *
+ * Try to lock tuple for update as part of speculative insertion. If
+ * a qual originating from ON CONFLICT DO UPDATE is satisfied, update
+ * (but still lock row, even though it may not satisfy estate's
+ * snapshot).
+ *
+ * Returns true if we're done (with or without an update), or false if
+ * the caller must retry the INSERT from scratch.
+ */
+static bool
+ExecOnConflictUpdate(ModifyTableContext *context,
+ ResultRelInfo *resultRelInfo,
+ ItemPointer conflictTid,
+ TupleTableSlot *excludedSlot,
+ bool canSetTag,
+ TupleTableSlot **returning)
+{
+ ModifyTableState *mtstate = context->mtstate;
+ ExprContext *econtext = mtstate->ps.ps_ExprContext;
+ Relation relation = resultRelInfo->ri_RelationDesc;
+ ExprState *onConflictSetWhere = resultRelInfo->ri_onConflict->oc_WhereClause;
+ TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing;
+ LockTupleMode lockmode;
+
+ /*
+ * Parse analysis should have blocked ON CONFLICT for all system
+ * relations, which includes these. There's no fundamental obstacle to
+ * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other
+ * ExecUpdate() caller.
+ */
+ Assert(!resultRelInfo->ri_needLockTagTuple);
+
+ /* Determine lock mode to use */
+ lockmode = ExecUpdateLockMode(context->estate, resultRelInfo);
+
+ /* Lock tuple for update. */
+ if (!ExecOnConflictLockRow(context, existing, conflictTid,
+ resultRelInfo->ri_RelationDesc, lockmode, true))
+ return false;
/*
* Verify that the tuple is visible to our MVCC snapshot if the current
@@ -2910,6 +2959,133 @@ ExecOnConflictUpdate(ModifyTableContext *context,
return true;
}
+/*
+ * ExecOnConflictSelect --- execute SELECT of INSERT ON CONFLICT DO SELECT
+ *
+ * Returns true if if we're done (with or without an update), or false if the
+ * caller must retry the INSERT from scratch.
+ */
+static bool
+ExecOnConflictSelect(ModifyTableContext *context,
+ ResultRelInfo *resultRelInfo,
+ ItemPointer conflictTid,
+ bool canSetTag,
+ TupleTableSlot **rslot)
+{
+ ModifyTableState *mtstate = context->mtstate;
+ ExprContext *econtext = mtstate->ps.ps_ExprContext;
+ Relation relation = resultRelInfo->ri_RelationDesc;
+ ExprState *onConflictSelectWhere = resultRelInfo->ri_onConflict->oc_WhereClause;
+ TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing;
+ LockClauseStrength lockstrength = resultRelInfo->ri_onConflict->oc_LockingStrength;
+
+ /*
+ * Parse analysis should have blocked ON CONFLICT for all system
+ * relations, which includes these. There's no fundamental obstacle to
+ * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other
+ * ExecUpdate() caller.
+ */
+ Assert(!resultRelInfo->ri_needLockTagTuple);
+
+ if (lockstrength != LCS_NONE)
+ {
+ LockTupleMode lockmode;
+
+ switch (lockstrength)
+ {
+ case LCS_FORKEYSHARE:
+ lockmode = LockTupleKeyShare;
+ break;
+ case LCS_FORSHARE:
+ lockmode = LockTupleShare;
+ break;
+ case LCS_FORNOKEYUPDATE:
+ lockmode = LockTupleNoKeyExclusive;
+ break;
+ case LCS_FORUPDATE:
+ lockmode = LockTupleExclusive;
+ break;
+ default:
+ elog(ERROR, "unexpected lock strength %d", lockstrength);
+ }
+
+ if (!ExecOnConflictLockRow(context, existing, conflictTid,
+ resultRelInfo->ri_RelationDesc, lockmode, false))
+ return false;
+ }
+ else
+ {
+ if (!table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing))
+ return false;
+ }
+
+ /*
+ * For the same reasons as ExecOnConflictUpdate, we must verify that the
+ * tuple is visible to our snapshot.
+ */
+ ExecCheckTupleVisible(context->estate, relation, existing);
+
+ /*
+ * Make the tuple available to ExecQual and ExecProject. EXCLUDED is not
+ * used at all.
+ */
+ econtext->ecxt_scantuple = existing;
+ econtext->ecxt_innertuple = NULL;
+ econtext->ecxt_outertuple = NULL;
+
+ if (!ExecQual(onConflictSelectWhere, econtext))
+ {
+ ExecClearTuple(existing); /* see return below */
+ InstrCountFiltered1(&mtstate->ps, 1);
+ return true; /* done with the tuple */
+ }
+
+ if (resultRelInfo->ri_WithCheckOptions != NIL)
+ {
+ /*
+ * Check target's existing tuple against UPDATE-applicable USING
+ * security barrier quals (if any), enforced here as RLS checks/WCOs.
+ *
+ * The rewriter creates UPDATE RLS checks/WCOs for UPDATE security
+ * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK,
+ * but that's almost the extent of its special handling for ON
+ * CONFLICT DO UPDATE.
+ *
+ * The rewriter will also have associated UPDATE applicable straight
+ * RLS checks/WCOs for the benefit of the ExecUpdate() call that
+ * follows. INSERTs and UPDATEs naturally have mutually exclusive WCO
+ * kinds, so there is no danger of spurious over-enforcement in the
+ * INSERT or UPDATE path.
+ */
+ ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
+ existing,
+ mtstate->ps.state);
+ }
+
+ /* Parse analysis should already have disallowed this */
+ Assert(resultRelInfo->ri_projectReturning);
+
+ *rslot = ExecProcessReturning(context, resultRelInfo, CMD_INSERT,
+ existing, NULL, context->planSlot);
+
+ if (canSetTag)
+ context->estate->es_processed++;
+
+ /*
+ * Before releasing the existing tuple, make sure rslot has a local copy
+ * of any pass-by-reference values.
+ */
+ ExecMaterializeSlot(*rslot);
+
+ /*
+ * Clear out existing tuple, as there might not be another conflict among
+ * the next input rows. Don't want to hold resources till the end of the
+ * query.
+ */
+ ExecClearTuple(existing);
+ return true;
+}
+
/*
* Perform MERGE.
*/
@@ -4915,6 +5091,34 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
onconfl->oc_WhereClause = qualexpr;
}
}
+ else if (node->onConflictAction == ONCONFLICT_SELECT)
+ {
+ OnConflictSetState *onconfl = makeNode(OnConflictSetState);
+
+ /* already exists if created by RETURNING processing above */
+ if (mtstate->ps.ps_ExprContext == NULL)
+ ExecAssignExprContext(estate, &mtstate->ps);
+
+ /* create state for DO SELECT operation */
+ resultRelInfo->ri_onConflict = onconfl;
+
+ /* initialize slot for the existing tuple */
+ onconfl->oc_Existing =
+ table_slot_create(resultRelInfo->ri_RelationDesc,
+ &mtstate->ps.state->es_tupleTable);
+
+ /* initialize state to evaluate the WHERE clause, if any */
+ if (node->onConflictWhere)
+ {
+ ExprState *qualexpr;
+
+ qualexpr = ExecInitQual((List *) node->onConflictWhere,
+ &mtstate->ps);
+ onconfl->oc_WhereClause = qualexpr;
+ }
+
+ onconfl->oc_LockingStrength = node->onConflictLockingStrength;
+ }
/*
* If we have any secondary relations in an UPDATE or DELETE, they need to
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 75e2b0b9036..523075a8f7d 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -7143,6 +7143,7 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
node->onConflictSet = NIL;
node->onConflictCols = NIL;
node->onConflictWhere = NULL;
+ node->onConflictLockingStrength = LCS_NONE;
node->arbiterIndexes = NIL;
node->exclRelRTI = 0;
node->exclRelTlist = NIL;
@@ -7161,6 +7162,7 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
node->onConflictCols =
extract_update_targetlist_colnos(node->onConflictSet);
node->onConflictWhere = onconflict->onConflictWhere;
+ node->onConflictLockingStrength = onconflict->lockingStrength;
/*
* If a set of unique index inference elements was provided (an
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 76f58b3aca3..806d2689f20 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -684,7 +684,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
ListCell *icols;
ListCell *attnos;
ListCell *lc;
- bool isOnConflictUpdate;
+ bool requiresUpdatePerm;
AclMode targetPerms;
/* There can't be any outer WITH to worry about */
@@ -703,8 +703,10 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
qry->override = stmt->override;
- isOnConflictUpdate = (stmt->onConflictClause &&
- stmt->onConflictClause->action == ONCONFLICT_UPDATE);
+ requiresUpdatePerm = (stmt->onConflictClause &&
+ (stmt->onConflictClause->action == ONCONFLICT_UPDATE ||
+ (stmt->onConflictClause->action == ONCONFLICT_SELECT &&
+ stmt->onConflictClause->lockingStrength != LCS_NONE)));
/*
* We have three cases to deal with: DEFAULT VALUES (selectStmt == NULL),
@@ -754,7 +756,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
* to the joinlist or namespace.
*/
targetPerms = ACL_INSERT;
- if (isOnConflictUpdate)
+ if (requiresUpdatePerm)
targetPerms |= ACL_UPDATE;
qry->resultRelation = setTargetTable(pstate, stmt->relation,
false, false, targetPerms);
@@ -1061,6 +1063,12 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
false, true, true);
}
+ if (stmt->onConflictClause && stmt->onConflictClause->action == ONCONFLICT_SELECT && !stmt->returningClause)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"),
+ parser_errposition(pstate, stmt->onConflictClause->location)));
+
/* Process ON CONFLICT, if any. */
if (stmt->onConflictClause)
qry->onConflict = transformOnConflictClause(pstate,
@@ -1287,8 +1295,15 @@ transformOnConflictClause(ParseState *pstate,
Assert((ParseNamespaceItem *) llast(pstate->p_namespace) == exclNSItem);
pstate->p_namespace = list_delete_last(pstate->p_namespace);
}
+ else if (onConflictClause->action == ONCONFLICT_SELECT)
+ {
+ onConflictWhere = transformWhereClause(pstate,
+ onConflictClause->whereClause,
+ EXPR_KIND_WHERE, "WHERE");
+
+ }
- /* Finally, build ON CONFLICT DO [NOTHING | UPDATE] expression */
+ /* Finally, build ON CONFLICT DO [NOTHING | SELECT | UPDATE] expression */
result = makeNode(OnConflictExpr);
result->action = onConflictClause->action;
@@ -1296,6 +1311,7 @@ transformOnConflictClause(ParseState *pstate,
result->arbiterWhere = arbiterWhere;
result->constraint = arbiterConstraint;
result->onConflictSet = onConflictSet;
+ result->lockingStrength = onConflictClause->lockingStrength;
result->onConflictWhere = onConflictWhere;
result->exclRelIndex = exclRelIndex;
result->exclRelTlist = exclRelTlist;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0fc502a3a40..7b80799142e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -473,7 +473,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <ival> OptNoLog
%type <oncommit> OnCommitOption
-%type <ival> for_locking_strength
+%type <ival> for_locking_strength opt_for_locking_strength
%type <node> for_locking_item
%type <list> for_locking_clause opt_for_locking_clause for_locking_items
%type <list> locked_rels_list
@@ -12317,12 +12317,24 @@ insert_column_item:
;
opt_on_conflict:
+ ON CONFLICT opt_conf_expr DO SELECT opt_for_locking_strength where_clause
+ {
+ $$ = makeNode(OnConflictClause);
+ $$->action = ONCONFLICT_SELECT;
+ $$->infer = $3;
+ $$->targetList = NIL;
+ $$->lockingStrength = $6;
+ $$->whereClause = $7;
+ $$->location = @1;
+ }
+ |
ON CONFLICT opt_conf_expr DO UPDATE SET set_clause_list where_clause
{
$$ = makeNode(OnConflictClause);
$$->action = ONCONFLICT_UPDATE;
$$->infer = $3;
$$->targetList = $7;
+ $$->lockingStrength = LCS_NONE;
$$->whereClause = $8;
$$->location = @1;
}
@@ -12333,6 +12345,7 @@ opt_on_conflict:
$$->action = ONCONFLICT_NOTHING;
$$->infer = $3;
$$->targetList = NIL;
+ $$->lockingStrength = LCS_NONE;
$$->whereClause = NULL;
$$->location = @1;
}
@@ -13569,6 +13582,11 @@ for_locking_strength:
| FOR KEY SHARE { $$ = LCS_FORKEYSHARE; }
;
+opt_for_locking_strength:
+ for_locking_strength { $$ = $1; }
+ | /* EMPTY */ { $$ = LCS_NONE; }
+ ;
+
locked_rels_list:
OF qualified_name_list { $$ = $2; }
| /* EMPTY */ { $$ = NIL; }
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 2e64fcae7b2..ab16e1495a3 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3312,6 +3312,13 @@ transformOnConflictArbiter(ParseState *pstate,
errhint("For example, ON CONFLICT (column_name)."),
parser_errposition(pstate,
exprLocation((Node *) onConflictClause))));
+ else if (onConflictClause->action == ONCONFLICT_SELECT && !infer)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ON CONFLICT DO SELECT requires inference specification or constraint name"),
+ errhint("For example, ON CONFLICT (column_name)."),
+ parser_errposition(pstate,
+ exprLocation((Node *) onConflictClause))));
/*
* To simplify certain aspects of its design, speculative insertion into
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 4dad384d04d..e2877faca91 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -301,11 +301,14 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
}
/*
- * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
- * checks for the UPDATE which may be applied to the same RTE.
+ * For INSERT ... ON CONFLICT DO UPDATE and DO SELECT FOR ... we need
+ * additional policy checks for the UPDATE or locking which may be
+ * applied to the same RTE.
*/
if (commandType == CMD_INSERT &&
- root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
+ root->onConflict && (root->onConflict->action == ONCONFLICT_UPDATE ||
+ (root->onConflict->action == ONCONFLICT_SELECT &&
+ root->onConflict->lockingStrength != LCS_NONE)))
{
List *conflict_permissive_policies;
List *conflict_restrictive_policies;
@@ -334,9 +337,9 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
/*
* Get and add ALL/SELECT policies, as WCO_RLS_CONFLICT_CHECK WCOs
* to ensure they are considered when taking the UPDATE path of an
- * INSERT .. ON CONFLICT DO UPDATE, if SELECT rights are required
- * for this relation, also as WCO policies, again, to avoid
- * silently dropping data. See above.
+ * INSERT .. ON CONFLICT, if SELECT rights are required for this
+ * relation, also as WCO policies, again, to avoid silently
+ * dropping data. See above.
*/
if (perminfo->requiredPerms & ACL_SELECT)
{
@@ -364,8 +367,8 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
/*
* Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure
* that the final updated row is visible when taking the UPDATE
- * path of an INSERT .. ON CONFLICT DO UPDATE, if SELECT rights
- * are required for this relation.
+ * path of an INSERT .. ON CONFLICT, if SELECT rights are required
+ * for this relation.
*/
if (perminfo->requiredPerms & ACL_SELECT)
add_with_check_options(rel, rt_index,
@@ -376,6 +379,29 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
hasSubLinks,
true);
}
+
+ /*
+ * For INSERT ... ON CONFLICT DO SELELT we need additional policy
+ * checks for the SELECT which may be applied to the same RTE.
+ */
+ if (commandType == CMD_INSERT &&
+ root->onConflict && root->onConflict->action == ONCONFLICT_SELECT &&
+ root->onConflict->lockingStrength == LCS_NONE)
+ {
+ List *conflict_permissive_policies;
+ List *conflict_restrictive_policies;
+
+ get_policies_for_relation(rel, CMD_SELECT, user_id,
+ &conflict_permissive_policies,
+ &conflict_restrictive_policies);
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_CONFLICT_CHECK,
+ conflict_permissive_policies,
+ conflict_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ true);
+ }
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 5b6cadb5a6c..95204f6c788 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -428,6 +428,8 @@ typedef struct OnConflictSetState
TupleTableSlot *oc_Existing; /* slot to store existing target tuple in */
TupleTableSlot *oc_ProjSlot; /* CONFLICT ... SET ... projection target */
ProjectionInfo *oc_ProjInfo; /* for ON CONFLICT DO UPDATE SET */
+ LockClauseStrength oc_LockingStrength; /* strengh of lock for ON CONFLICT
+ * DO SELECT, or LCS_NONE */
ExprState *oc_WhereClause; /* state for the WHERE clause */
} OnConflictSetState;
diff --git a/src/include/nodes/lockoptions.h b/src/include/nodes/lockoptions.h
index 0b534e30603..59434fd480e 100644
--- a/src/include/nodes/lockoptions.h
+++ b/src/include/nodes/lockoptions.h
@@ -20,7 +20,8 @@
*/
typedef enum LockClauseStrength
{
- LCS_NONE, /* no such clause - only used in PlanRowMark */
+ LCS_NONE, /* no such clause - only used in PlanRowMark
+ * and ON CONFLICT SELECT */
LCS_FORKEYSHARE, /* FOR KEY SHARE */
LCS_FORSHARE, /* FOR SHARE */
LCS_FORNOKEYUPDATE, /* FOR NO KEY UPDATE */
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index fbe333d88fa..e6d6a3bf847 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -424,6 +424,7 @@ typedef enum OnConflictAction
ONCONFLICT_NONE, /* No "ON CONFLICT" clause */
ONCONFLICT_NOTHING, /* ON CONFLICT ... DO NOTHING */
ONCONFLICT_UPDATE, /* ON CONFLICT ... DO UPDATE */
+ ONCONFLICT_SELECT, /* ON CONFLICT ... DO SELECT */
} OnConflictAction;
/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index df331b1c0d9..db39240d2f3 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1635,9 +1635,11 @@ typedef struct InferClause
typedef struct OnConflictClause
{
NodeTag type;
- OnConflictAction action; /* DO NOTHING or UPDATE? */
+ OnConflictAction action; /* DO NOTHING, SELECT or UPDATE? */
InferClause *infer; /* Optional index inference clause */
List *targetList; /* the target list (of ResTarget) */
+ LockClauseStrength lockingStrength; /* strengh of lock for DO SELECT, or
+ * LCS_NONE */
Node *whereClause; /* qualifications */
ParseLoc location; /* token location, or -1 if unknown */
} OnConflictClause;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 658d76225e4..1ad0a240b38 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -322,6 +322,8 @@ typedef struct ModifyTable
OnConflictAction onConflictAction;
/* List of ON CONFLICT arbiter index OIDs */
List *arbiterIndexes;
+ /* lock strength for ON CONFLICT SELECT */
+ LockClauseStrength onConflictLockingStrength;
/* INSERT ON CONFLICT DO UPDATE targetlist */
List *onConflictSet;
/* target column numbers for onConflictSet */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 7d3b4198f26..1a2a183b5bd 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -21,6 +21,7 @@
#include "access/cmptype.h"
#include "nodes/bitmapset.h"
#include "nodes/pg_list.h"
+#include "nodes/lockoptions.h"
typedef enum OverridingKind
@@ -2358,9 +2359,15 @@ typedef struct OnConflictExpr
Node *arbiterWhere; /* unique index arbiter WHERE clause */
Oid constraint; /* pg_constraint OID for arbiter */
+ /* both ON CONFLICT SELECT and UPDATE */
+ Node *onConflictWhere; /* qualifiers to restrict SELECT/UPDATE to */
+
+ /* ON CONFLICT SELECT */
+ LockClauseStrength lockingStrength; /* strengh of lock for DO SELECT, or
+ * LCS_NONE */
+
/* ON CONFLICT UPDATE */
List *onConflictSet; /* List of ON CONFLICT SET TargetEntrys */
- Node *onConflictWhere; /* qualifiers to restrict UPDATE to */
int exclRelIndex; /* RT index of 'excluded' relation */
List *exclRelTlist; /* tlist of the EXCLUDED pseudo relation */
} OnConflictExpr;
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index fdd0f6c8f25..2edf04c78f3 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -249,6 +249,68 @@ insert into insertconflicttest values (2, 'Orange') on conflict (key, key, key)
insert into insertconflicttest
values (1, 'Apple'), (2, 'Orange')
on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key);
+-- DO SELECT
+delete from insertconflicttest where fruit = 'Apple';
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select; -- fails
+ERROR: ON CONFLICT DO SELECT requires a RETURNING clause
+LINE 1: ...nsert into insertconflicttest values (1, 'Apple') on conflic...
+ ^
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select where fruit <> 'Apple' returning *;
+ key | fruit
+-----+-------
+(0 rows)
+
+delete from insertconflicttest where fruit = 'Apple';
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update where fruit <> 'Apple' returning *;
+ key | fruit
+-----+-------
+(0 rows)
+
+insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*;
+ key | fruit | key | fruit | key | fruit
+-----+-------+-----+-------+-----+-------
+ | | 3 | Pear | 3 | Pear
+(1 row)
+
+insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*;
+ key | fruit | key | fruit | key | fruit
+-----+-------+-----+-------+-----+-------
+ 3 | Pear | | | 3 | Pear
+(1 row)
+
+explain (costs off) insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for key share returning *;
+ QUERY PLAN
+---------------------------------------------
+ Insert on insertconflicttest
+ Conflict Resolution: SELECT FOR KEY SHARE
+ Conflict Arbiter Indexes: key_index
+ -> Result
+(4 rows)
+
-- Give good diagnostic message when EXCLUDED.* spuriously referenced from
-- RETURNING:
insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit;
@@ -269,26 +331,26 @@ LINE 1: ... 'Apple') on conflict (key) do update set fruit = excluded.f...
^
HINT: Perhaps you meant to reference the column "excluded.fruit".
-- inference fails:
-insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (4, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (5, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (5, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (6, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (6, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (7, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-- Check the target relation can be aliased
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name
ERROR: invalid reference to FROM-clause entry for table "insertconflicttest"
LINE 1: ...onfruit') on conflict (key) do update set fruit = insertconf...
^
HINT: Perhaps you meant to reference the table alias "ict".
-- Check helpful hint when qualifying set column with target table
-insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango';
+insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango';
ERROR: column "insertconflicttest" of relation "insertconflicttest" does not exist
-LINE 1: ...3, 'Kiwi') on conflict (key, fruit) do update set insertconf...
+LINE 1: ...4, 'Kiwi') on conflict (key, fruit) do update set insertconf...
^
HINT: SET target columns cannot be qualified with the relation name.
drop index key_index;
@@ -297,16 +359,16 @@ drop index key_index;
--
create unique index comp_key_index on insertconflicttest(key, fruit);
-- inference succeeds:
-insert into insertconflicttest values (7, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (8, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (8, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (9, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit;
-- inference fails:
-insert into insertconflicttest values (9, 'Banana') on conflict (key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (10, 'Banana') on conflict (key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (10, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (11, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (11, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (12, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (12, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (13, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
drop index comp_key_index;
--
@@ -315,17 +377,17 @@ drop index comp_key_index;
create unique index part_comp_key_index on insertconflicttest(key, fruit) where key < 5;
create unique index expr_part_comp_key_index on insertconflicttest(key, lower(fruit)) where key < 5;
-- inference fails:
-insert into insertconflicttest values (13, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (14, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (14, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (15, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (15, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (16, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (16, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (17, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (17, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (18, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
-insert into insertconflicttest values (18, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (19, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
drop index part_comp_key_index;
drop index expr_part_comp_key_index;
@@ -735,13 +797,58 @@ insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 =
ERROR: ON CONFLICT DO UPDATE command cannot affect row a second time
HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
commit;
+begin transaction isolation level read committed;
+insert into selfconflict values (7,1), (7,2) on conflict(f1) do select returning *;
+ f1 | f2
+----+----
+ 7 | 1
+ 7 | 1
+(2 rows)
+
+commit;
+begin transaction isolation level repeatable read;
+insert into selfconflict values (8,1), (8,2) on conflict(f1) do select returning *;
+ f1 | f2
+----+----
+ 8 | 1
+ 8 | 1
+(2 rows)
+
+commit;
+begin transaction isolation level serializable;
+insert into selfconflict values (9,1), (9,2) on conflict(f1) do select returning *;
+ f1 | f2
+----+----
+ 9 | 1
+ 9 | 1
+(2 rows)
+
+commit;
+begin transaction isolation level read committed;
+insert into selfconflict values (10,1), (10,2) on conflict(f1) do select for update returning *;
+ERROR: ON CONFLICT DO SELECT command cannot affect row a second time
+HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
+begin transaction isolation level repeatable read;
+insert into selfconflict values (11,1), (11,2) on conflict(f1) do select for update returning *;
+ERROR: ON CONFLICT DO SELECT command cannot affect row a second time
+HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
+begin transaction isolation level serializable;
+insert into selfconflict values (12,1), (12,2) on conflict(f1) do select for update returning *;
+ERROR: ON CONFLICT DO SELECT command cannot affect row a second time
+HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
select * from selfconflict;
f1 | f2
----+----
1 | 1
2 | 1
3 | 1
-(3 rows)
+ 7 | 1
+ 8 | 1
+ 9 | 1
+(6 rows)
drop table selfconflict;
-- check ON CONFLICT handling with partitioned tables
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 549c46452ec..b80b7dae91a 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -101,6 +101,21 @@ insert into insertconflicttest
values (1, 'Apple'), (2, 'Orange')
on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key);
+-- DO SELECT
+delete from insertconflicttest where fruit = 'Apple';
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select; -- fails
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select where fruit <> 'Apple' returning *;
+delete from insertconflicttest where fruit = 'Apple';
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
+insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update where fruit <> 'Apple' returning *;
+insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*;
+insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*;
+
+explain (costs off) insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for key share returning *;
+
-- Give good diagnostic message when EXCLUDED.* spuriously referenced from
-- RETURNING:
insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit;
@@ -112,18 +127,18 @@ insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do update
insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruitt;
-- inference fails:
-insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (4, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (5, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (6, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (5, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (6, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (7, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit;
-- Check the target relation can be aliased
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias
-insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias
+insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name
-- Check helpful hint when qualifying set column with target table
-insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango';
+insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango';
drop index key_index;
@@ -133,14 +148,14 @@ drop index key_index;
create unique index comp_key_index on insertconflicttest(key, fruit);
-- inference succeeds:
-insert into insertconflicttest values (7, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (8, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (8, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (9, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit;
-- inference fails:
-insert into insertconflicttest values (9, 'Banana') on conflict (key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (10, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (11, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (12, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (10, 'Banana') on conflict (key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (11, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (12, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (13, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
drop index comp_key_index;
@@ -151,12 +166,12 @@ create unique index part_comp_key_index on insertconflicttest(key, fruit) where
create unique index expr_part_comp_key_index on insertconflicttest(key, lower(fruit)) where key < 5;
-- inference fails:
-insert into insertconflicttest values (13, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (14, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (15, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (16, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (17, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
-insert into insertconflicttest values (18, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (14, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (15, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (16, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (17, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (18, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit;
+insert into insertconflicttest values (19, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit;
drop index part_comp_key_index;
drop index expr_part_comp_key_index;
@@ -454,6 +469,30 @@ begin transaction isolation level serializable;
insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 = 0;
commit;
+begin transaction isolation level read committed;
+insert into selfconflict values (7,1), (7,2) on conflict(f1) do select returning *;
+commit;
+
+begin transaction isolation level repeatable read;
+insert into selfconflict values (8,1), (8,2) on conflict(f1) do select returning *;
+commit;
+
+begin transaction isolation level serializable;
+insert into selfconflict values (9,1), (9,2) on conflict(f1) do select returning *;
+commit;
+
+begin transaction isolation level read committed;
+insert into selfconflict values (10,1), (10,2) on conflict(f1) do select for update returning *;
+commit;
+
+begin transaction isolation level repeatable read;
+insert into selfconflict values (11,1), (11,2) on conflict(f1) do select for update returning *;
+commit;
+
+begin transaction isolation level serializable;
+insert into selfconflict values (12,1), (12,2) on conflict(f1) do select for update returning *;
+commit;
+
select * from selfconflict;
drop table selfconflict;
--
2.43.0
From 1df023e0e190d111a119f206e1f017cda163267c Mon Sep 17 00:00:00 2001
From: Dean Rasheed <dean.a.rash...@gmail.com>
Date: Mon, 31 Mar 2025 15:20:47 +0100
Subject: [PATCH v7 2/2] Review comments for ON CONFLICT DO SELECT.
---
doc/src/sgml/ref/create_policy.sgml | 16 +++
src/backend/commands/explain.c | 11 +-
src/backend/executor/nodeModifyTable.c | 61 ++++----
src/backend/optimizer/plan/setrefs.c | 3 +-
src/backend/parser/analyze.c | 60 ++++----
src/backend/rewrite/rewriteHandler.c | 13 ++
src/backend/rewrite/rowsecurity.c | 131 ++++++++----------
src/backend/utils/adt/ruleutils.c | 69 +++++----
src/include/nodes/plannodes.h | 2 +-
src/test/regress/expected/insert_conflict.out | 40 +++++-
src/test/regress/expected/rules.out | 55 ++++++++
src/test/regress/sql/insert_conflict.sql | 10 +-
src/test/regress/sql/rules.sql | 26 ++++
13 files changed, 336 insertions(+), 161 deletions(-)
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
index e76c342d3da..abbf1f23168 100644
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -491,6 +491,22 @@ CREATE POLICY <replaceable class="parameter">name</replaceable> ON <replaceable
<entry>New row</entry>
<entry>—</entry>
</row>
+ <row>
+ <entry><command>ON CONFLICT DO SELECT</command></entry>
+ <entry>Existing & new rows</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>ON CONFLICT DO SELECT FOR UPDATE/SHARE</command></entry>
+ <entry>Existing & new rows</entry>
+ <entry>—</entry>
+ <entry>Existing row</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ecb0bbeaa9c..8f9e63888b2 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4651,7 +4651,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
if (node->onConflictAction != ONCONFLICT_NONE)
{
- const char *resolution;
+ const char *resolution = NULL;
if (node->onConflictAction == ONCONFLICT_NOTHING)
resolution = "NOTHING";
@@ -4659,6 +4659,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
resolution = "UPDATE";
else
{
+ Assert(node->onConflictAction == ONCONFLICT_SELECT);
switch (node->onConflictLockingStrength)
{
case LCS_NONE:
@@ -4673,9 +4674,13 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
case LCS_FORNOKEYUPDATE:
resolution = "SELECT FOR NO KEY UPDATE";
break;
- default: /* LCS_FORUPDATE */
+ case LCS_FORUPDATE:
resolution = "SELECT FOR UPDATE";
break;
+ default:
+ elog(ERROR, "unrecognized LockClauseStrength %d",
+ (int) node->onConflictLockingStrength);
+ break;
}
}
@@ -4688,7 +4693,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
if (idxNames)
ExplainPropertyList("Conflict Arbiter Indexes", idxNames, es);
- /* ON CONFLICT DO UPDATE WHERE qual is specially displayed */
+ /* ON CONFLICT DO UPDATE/SELECT WHERE qual is specially displayed */
if (node->onConflictWhere)
{
show_upper_qual((List *) node->onConflictWhere, "Conflict Filter",
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 250685e2eda..ba5b0b4c363 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -160,6 +160,7 @@ static bool ExecOnConflictUpdate(ModifyTableContext *context,
static bool ExecOnConflictSelect(ModifyTableContext *context,
ResultRelInfo *resultRelInfo,
ItemPointer conflictTid,
+ TupleTableSlot *excludedSlot,
bool canSetTag,
TupleTableSlot **returning);
static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate,
@@ -1154,13 +1155,13 @@ ExecInsert(ModifyTableContext *context,
/*
* In case of ON CONFLICT DO SELECT, optionally lock the
* conflicting tuple, fetch it and project RETURNING on
- * it. Be prepared to retry if fetching fails because of a
+ * it. Be prepared to retry if locking fails because of a
* concurrent UPDATE/DELETE to the conflict tuple.
*/
TupleTableSlot *returning = NULL;
if (ExecOnConflictSelect(context, resultRelInfo,
- &conflictTid, canSetTag,
+ &conflictTid, slot, canSetTag,
&returning))
{
InstrCountTuples2(&mtstate->ps, 1);
@@ -2708,6 +2709,12 @@ redo_act:
/*
* ExecOnConflictLockRow --- lock the row for ON CONFLICT DO UPDATE/SELECT
+ *
+ * Try to lock tuple for update as part of speculative insertion for ON
+ * CONFLICT DO UPDATE or ON CONFLICT DO SELECT FOR UPDATE/SHARE.
+ *
+ * Returns true if the row is successfully locked, or false if the caller must
+ * retry the INSERT from scratch.
*/
static bool
ExecOnConflictLockRow(ModifyTableContext *context,
@@ -2865,7 +2872,7 @@ ExecOnConflictUpdate(ModifyTableContext *context,
/* Determine lock mode to use */
lockmode = ExecUpdateLockMode(context->estate, resultRelInfo);
- /* Lock tuple for update. */
+ /* Lock tuple for update */
if (!ExecOnConflictLockRow(context, existing, conflictTid,
resultRelInfo->ri_RelationDesc, lockmode, true))
return false;
@@ -2910,11 +2917,12 @@ ExecOnConflictUpdate(ModifyTableContext *context,
* security barrier quals (if any), enforced here as RLS checks/WCOs.
*
* The rewriter creates UPDATE RLS checks/WCOs for UPDATE security
- * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK,
- * but that's almost the extent of its special handling for ON
- * CONFLICT DO UPDATE.
+ * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK. If
+ * SELECT rights are required on the target table, the rewriter also
+ * adds SELECT RLS checks/WCOs for SELECT security quals, using WCOs
+ * of the same kind, so this check enforces them too.
*
- * The rewriter will also have associated UPDATE applicable straight
+ * The rewriter will also have associated UPDATE-applicable straight
* RLS checks/WCOs for the benefit of the ExecUpdate() call that
* follows. INSERTs and UPDATEs naturally have mutually exclusive WCO
* kinds, so there is no danger of spurious over-enforcement in the
@@ -2962,13 +2970,18 @@ ExecOnConflictUpdate(ModifyTableContext *context,
/*
* ExecOnConflictSelect --- execute SELECT of INSERT ON CONFLICT DO SELECT
*
- * Returns true if if we're done (with or without an update), or false if the
+ * If SELECT FOR UPDATE/SHARE is specified, try to lock tuple as part of
+ * speculative insertion. If a qual originating from ON CONFLICT DO UPDATE is
+ * satisfied, select the row.
+ *
+ * Returns true if if we're done (with or without a select), or false if the
* caller must retry the INSERT from scratch.
*/
static bool
ExecOnConflictSelect(ModifyTableContext *context,
ResultRelInfo *resultRelInfo,
ItemPointer conflictTid,
+ TupleTableSlot *excludedSlot,
bool canSetTag,
TupleTableSlot **rslot)
{
@@ -3026,11 +3039,13 @@ ExecOnConflictSelect(ModifyTableContext *context,
ExecCheckTupleVisible(context->estate, relation, existing);
/*
- * Make the tuple available to ExecQual and ExecProject. EXCLUDED is not
- * used at all.
+ * Make tuple and any needed join variables available to ExecQual. The
+ * EXCLUDED tuple is installed in ecxt_innertuple, while the target's
+ * existing tuple is installed in the scantuple. EXCLUDED has been made
+ * to reference INNER_VAR in setrefs.c, but there is no other redirection.
*/
econtext->ecxt_scantuple = existing;
- econtext->ecxt_innertuple = NULL;
+ econtext->ecxt_innertuple = excludedSlot;
econtext->ecxt_outertuple = NULL;
if (!ExecQual(onConflictSelectWhere, econtext))
@@ -3043,19 +3058,15 @@ ExecOnConflictSelect(ModifyTableContext *context,
if (resultRelInfo->ri_WithCheckOptions != NIL)
{
/*
- * Check target's existing tuple against UPDATE-applicable USING
+ * Check target's existing tuple against SELECT-applicable USING
* security barrier quals (if any), enforced here as RLS checks/WCOs.
*
- * The rewriter creates UPDATE RLS checks/WCOs for UPDATE security
- * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK,
- * but that's almost the extent of its special handling for ON
- * CONFLICT DO UPDATE.
- *
- * The rewriter will also have associated UPDATE applicable straight
- * RLS checks/WCOs for the benefit of the ExecUpdate() call that
- * follows. INSERTs and UPDATEs naturally have mutually exclusive WCO
- * kinds, so there is no danger of spurious over-enforcement in the
- * INSERT or UPDATE path.
+ * The rewriter creates SELECT RLS checks/WCOs for SELECT security
+ * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK. If
+ * FOR UPDATE/SHARE was specified, UPDATE rights are required on the
+ * target table, and the rewriter also adds UPDATE RLS checks/WCOs for
+ * UPDATE security quals, using WCOs of the same kind, so this check
+ * enforces them too.
*/
ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
existing,
@@ -3065,8 +3076,9 @@ ExecOnConflictSelect(ModifyTableContext *context,
/* Parse analysis should already have disallowed this */
Assert(resultRelInfo->ri_projectReturning);
- *rslot = ExecProcessReturning(context, resultRelInfo, CMD_INSERT,
- existing, NULL, context->planSlot);
+ /* Process RETURNING like an UPDATE that didn't change anything */
+ *rslot = ExecProcessReturning(context, resultRelInfo, CMD_UPDATE,
+ existing, existing, context->planSlot);
if (canSetTag)
context->estate->es_processed++;
@@ -3083,6 +3095,7 @@ ExecOnConflictSelect(ModifyTableContext *context,
* query.
*/
ExecClearTuple(existing);
+
return true;
}
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 150e9f060ee..9f87a0e7148 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1114,7 +1114,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
* those are already used by RETURNING and it seems better to
* be non-conflicting.
*/
- if (splan->onConflictSet)
+ if (splan->onConflictAction == ONCONFLICT_UPDATE ||
+ splan->onConflictAction == ONCONFLICT_SELECT)
{
indexed_tlist *itlist;
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 806d2689f20..21a2aed1f08 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1063,11 +1063,14 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
false, true, true);
}
- if (stmt->onConflictClause && stmt->onConflictClause->action == ONCONFLICT_SELECT && !stmt->returningClause)
+ /* ON CONFLICT DO SELECT requires a RETURNING clause */
+ if (stmt->onConflictClause &&
+ stmt->onConflictClause->action == ONCONFLICT_SELECT &&
+ !stmt->returningClause)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"),
- parser_errposition(pstate, stmt->onConflictClause->location)));
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"),
+ parser_errposition(pstate, stmt->onConflictClause->location));
/* Process ON CONFLICT, if any. */
if (stmt->onConflictClause)
@@ -1227,12 +1230,13 @@ transformOnConflictClause(ParseState *pstate,
OnConflictExpr *result;
/*
- * If this is ON CONFLICT ... UPDATE, first create the range table entry
- * for the EXCLUDED pseudo relation, so that that will be present while
- * processing arbiter expressions. (You can't actually reference it from
- * there, but this provides a useful error message if you try.)
+ * If this is ON CONFLICT ... UPDATE/SELECT, first create the range table
+ * entry for the EXCLUDED pseudo relation, so that that will be present
+ * while processing arbiter expressions. (You can't actually reference it
+ * from there, but this provides a useful error message if you try.)
*/
- if (onConflictClause->action == ONCONFLICT_UPDATE)
+ if (onConflictClause->action == ONCONFLICT_UPDATE ||
+ onConflictClause->action == ONCONFLICT_SELECT)
{
Relation targetrel = pstate->p_target_relation;
RangeTblEntry *exclRte;
@@ -1261,27 +1265,28 @@ transformOnConflictClause(ParseState *pstate,
transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
&arbiterWhere, &arbiterConstraint);
- /* Process DO UPDATE */
- if (onConflictClause->action == ONCONFLICT_UPDATE)
+ /* Process DO UPDATE/SELECT */
+ if (onConflictClause->action == ONCONFLICT_UPDATE ||
+ onConflictClause->action == ONCONFLICT_SELECT)
{
- /*
- * Expressions in the UPDATE targetlist need to be handled like UPDATE
- * not INSERT. We don't need to save/restore this because all INSERT
- * expressions have been parsed already.
- */
- pstate->p_is_insert = false;
-
/*
* Add the EXCLUDED pseudo relation to the query namespace, making it
- * available in the UPDATE subexpressions.
+ * available in the UPDATE/SELECT subexpressions.
*/
addNSItemToQuery(pstate, exclNSItem, false, true, true);
- /*
- * Now transform the UPDATE subexpressions.
- */
- onConflictSet =
- transformUpdateTargetList(pstate, onConflictClause->targetList);
+ if (onConflictClause->action == ONCONFLICT_UPDATE)
+ {
+ /*
+ * Expressions in the UPDATE targetlist need to be handled like
+ * UPDATE not INSERT. We don't need to save/restore this because
+ * all INSERT expressions have been parsed already.
+ */
+ pstate->p_is_insert = false;
+
+ onConflictSet =
+ transformUpdateTargetList(pstate, onConflictClause->targetList);
+ }
onConflictWhere = transformWhereClause(pstate,
onConflictClause->whereClause,
@@ -1295,13 +1300,6 @@ transformOnConflictClause(ParseState *pstate,
Assert((ParseNamespaceItem *) llast(pstate->p_namespace) == exclNSItem);
pstate->p_namespace = list_delete_last(pstate->p_namespace);
}
- else if (onConflictClause->action == ONCONFLICT_SELECT)
- {
- onConflictWhere = transformWhereClause(pstate,
- onConflictClause->whereClause,
- EXPR_KIND_WHERE, "WHERE");
-
- }
/* Finally, build ON CONFLICT DO [NOTHING | SELECT | UPDATE] expression */
result = makeNode(OnConflictExpr);
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index f0bce5f9ed9..a0fa66eaada 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -655,6 +655,19 @@ rewriteRuleAction(Query *parsetree,
rule_action = sub_action;
}
+ /*
+ * If rule_action is INSERT .. ON CONFLICT DO SELECT, the parser should
+ * have verified that it has a RETURNING clause, but we must also check
+ * that the triggering query has a RETURNING clause.
+ */
+ if (rule_action->onConflict &&
+ rule_action->onConflict->action == ONCONFLICT_SELECT &&
+ (!rule_action->returningList || !parsetree->returningList))
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"),
+ errdetail("A rule action is INSERT ... ON CONFLICT DO SELECT, which requires a RETURNING clause"));
+
/*
* If rule_action has a RETURNING clause, then either throw it away if the
* triggering query has no RETURNING clause, or rewrite it to emit what
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index e2877faca91..c9bdff6f8f5 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -301,45 +301,50 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
}
/*
- * For INSERT ... ON CONFLICT DO UPDATE and DO SELECT FOR ... we need
- * additional policy checks for the UPDATE or locking which may be
- * applied to the same RTE.
+ * For INSERT ... ON CONFLICT DO UPDATE/SELECT we need additional
+ * policy checks for the UPDATE/SELECT which may be applied to the
+ * same RTE.
*/
- if (commandType == CMD_INSERT &&
- root->onConflict && (root->onConflict->action == ONCONFLICT_UPDATE ||
- (root->onConflict->action == ONCONFLICT_SELECT &&
- root->onConflict->lockingStrength != LCS_NONE)))
+ if (commandType == CMD_INSERT && root->onConflict &&
+ (root->onConflict->action == ONCONFLICT_UPDATE ||
+ root->onConflict->action == ONCONFLICT_SELECT))
{
- List *conflict_permissive_policies;
- List *conflict_restrictive_policies;
+ List *conflict_permissive_policies = NIL;
+ List *conflict_restrictive_policies = NIL;
List *conflict_select_permissive_policies = NIL;
List *conflict_select_restrictive_policies = NIL;
- /* Get the policies that apply to the auxiliary UPDATE */
- get_policies_for_relation(rel, CMD_UPDATE, user_id,
- &conflict_permissive_policies,
- &conflict_restrictive_policies);
-
- /*
- * Enforce the USING clauses of the UPDATE policies using WCOs
- * rather than security quals. This ensures that an error is
- * raised if the conflicting row cannot be updated due to RLS,
- * rather than the change being silently dropped.
- */
- add_with_check_options(rel, rt_index,
- WCO_RLS_CONFLICT_CHECK,
- conflict_permissive_policies,
- conflict_restrictive_policies,
- withCheckOptions,
- hasSubLinks,
- true);
+ if (perminfo->requiredPerms & ACL_UPDATE)
+ {
+ /*
+ * Get the policies that apply to the auxiliary UPDATE or
+ * SELECT FOR SHARE/UDPATE.
+ */
+ get_policies_for_relation(rel, CMD_UPDATE, user_id,
+ &conflict_permissive_policies,
+ &conflict_restrictive_policies);
+
+ /*
+ * Enforce the USING clauses of the UPDATE policies using WCOs
+ * rather than security quals. This ensures that an error is
+ * raised if the conflicting row cannot be updated/locked due
+ * to RLS, rather than the change being silently dropped.
+ */
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_CONFLICT_CHECK,
+ conflict_permissive_policies,
+ conflict_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ true);
+ }
/*
* Get and add ALL/SELECT policies, as WCO_RLS_CONFLICT_CHECK WCOs
- * to ensure they are considered when taking the UPDATE path of an
- * INSERT .. ON CONFLICT, if SELECT rights are required for this
- * relation, also as WCO policies, again, to avoid silently
- * dropping data. See above.
+ * to ensure they are considered when taking the UPDATE/SELECT
+ * path of an INSERT .. ON CONFLICT, if SELECT rights are required
+ * for this relation, also as WCO policies, again, to avoid
+ * silently dropping data. See above.
*/
if (perminfo->requiredPerms & ACL_SELECT)
{
@@ -355,52 +360,36 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
true);
}
- /* Enforce the WITH CHECK clauses of the UPDATE policies */
- add_with_check_options(rel, rt_index,
- WCO_RLS_UPDATE_CHECK,
- conflict_permissive_policies,
- conflict_restrictive_policies,
- withCheckOptions,
- hasSubLinks,
- false);
-
/*
- * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure
- * that the final updated row is visible when taking the UPDATE
- * path of an INSERT .. ON CONFLICT, if SELECT rights are required
- * for this relation.
+ * For INSERT .. ON CONFLICT DO UPDATE, add additional policies to
+ * be checked when the auxiliary UPDATE is executed.
*/
- if (perminfo->requiredPerms & ACL_SELECT)
+ if (root->onConflict->action == ONCONFLICT_UPDATE)
+ {
+ /* Enforce the WITH CHECK clauses of the UPDATE policies */
add_with_check_options(rel, rt_index,
WCO_RLS_UPDATE_CHECK,
- conflict_select_permissive_policies,
- conflict_select_restrictive_policies,
+ conflict_permissive_policies,
+ conflict_restrictive_policies,
withCheckOptions,
hasSubLinks,
- true);
- }
-
- /*
- * For INSERT ... ON CONFLICT DO SELELT we need additional policy
- * checks for the SELECT which may be applied to the same RTE.
- */
- if (commandType == CMD_INSERT &&
- root->onConflict && root->onConflict->action == ONCONFLICT_SELECT &&
- root->onConflict->lockingStrength == LCS_NONE)
- {
- List *conflict_permissive_policies;
- List *conflict_restrictive_policies;
-
- get_policies_for_relation(rel, CMD_SELECT, user_id,
- &conflict_permissive_policies,
- &conflict_restrictive_policies);
- add_with_check_options(rel, rt_index,
- WCO_RLS_CONFLICT_CHECK,
- conflict_permissive_policies,
- conflict_restrictive_policies,
- withCheckOptions,
- hasSubLinks,
- true);
+ false);
+
+ /*
+ * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to
+ * ensure that the final updated row is visible when taking
+ * the UPDATE path of an INSERT .. ON CONFLICT, if SELECT
+ * rights are required for this relation.
+ */
+ if (perminfo->requiredPerms & ACL_SELECT)
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_UPDATE_CHECK,
+ conflict_select_permissive_policies,
+ conflict_select_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ true);
+ }
}
}
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 9e90acedb91..0e95c750fa3 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -426,6 +426,7 @@ static void get_update_query_targetlist_def(Query *query, List *targetList,
static void get_delete_query_def(Query *query, deparse_context *context);
static void get_merge_query_def(Query *query, deparse_context *context);
static void get_utility_query_def(Query *query, deparse_context *context);
+static char *get_lock_clause_strength(LockClauseStrength strength);
static void get_basic_select_query(Query *query, deparse_context *context);
static void get_target_list(List *targetList, deparse_context *context);
static void get_returning_clause(Query *query, deparse_context *context);
@@ -5984,30 +5985,9 @@ get_select_query_def(Query *query, deparse_context *context)
if (rc->pushedDown)
continue;
- switch (rc->strength)
- {
- case LCS_NONE:
- /* we intentionally throw an error for LCS_NONE */
- elog(ERROR, "unrecognized LockClauseStrength %d",
- (int) rc->strength);
- break;
- case LCS_FORKEYSHARE:
- appendContextKeyword(context, " FOR KEY SHARE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- break;
- case LCS_FORSHARE:
- appendContextKeyword(context, " FOR SHARE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- break;
- case LCS_FORNOKEYUPDATE:
- appendContextKeyword(context, " FOR NO KEY UPDATE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- break;
- case LCS_FORUPDATE:
- appendContextKeyword(context, " FOR UPDATE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- break;
- }
+ appendContextKeyword(context,
+ get_lock_clause_strength(rc->strength),
+ -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
appendStringInfo(buf, " OF %s",
quote_identifier(get_rtable_name(rc->rti,
@@ -6020,6 +6000,28 @@ get_select_query_def(Query *query, deparse_context *context)
}
}
+static char *
+get_lock_clause_strength(LockClauseStrength strength)
+{
+ switch (strength)
+ {
+ case LCS_NONE:
+ /* we intentionally throw an error for LCS_NONE */
+ elog(ERROR, "unrecognized LockClauseStrength %d",
+ (int) strength);
+ break;
+ case LCS_FORKEYSHARE:
+ return " FOR KEY SHARE";
+ case LCS_FORSHARE:
+ return " FOR SHARE";
+ case LCS_FORNOKEYUPDATE:
+ return " FOR NO KEY UPDATE";
+ case LCS_FORUPDATE:
+ return " FOR UPDATE";
+ }
+ return NULL; /* keep compiler quiet */
+}
+
/*
* Detect whether query looks like SELECT ... FROM VALUES(),
* with no need to rename the output columns of the VALUES RTE.
@@ -7110,7 +7112,7 @@ get_insert_query_def(Query *query, deparse_context *context)
{
appendStringInfoString(buf, " DO NOTHING");
}
- else
+ else if (confl->action == ONCONFLICT_UPDATE)
{
appendStringInfoString(buf, " DO UPDATE SET ");
/* Deparse targetlist */
@@ -7125,6 +7127,23 @@ get_insert_query_def(Query *query, deparse_context *context)
get_rule_expr(confl->onConflictWhere, context, false);
}
}
+ else
+ {
+ Assert(confl->action == ONCONFLICT_SELECT);
+ appendStringInfoString(buf, " DO SELECT");
+
+ /* Add FOR [KEY] UPDATE/SHARE clause if present */
+ if (confl->lockingStrength != LCS_NONE)
+ appendStringInfoString(buf, get_lock_clause_strength(confl->lockingStrength));
+
+ /* Add a WHERE clause if given */
+ if (confl->onConflictWhere != NULL)
+ {
+ appendContextKeyword(context, " WHERE ",
+ -PRETTYINDENT_STD, PRETTYINDENT_STD, 1);
+ get_rule_expr(confl->onConflictWhere, context, false);
+ }
+ }
}
/* Add RETURNING if present */
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 1ad0a240b38..1743bc22e08 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -328,7 +328,7 @@ typedef struct ModifyTable
List *onConflictSet;
/* target column numbers for onConflictSet */
List *onConflictCols;
- /* WHERE for ON CONFLICT UPDATE */
+ /* WHERE for ON CONFLICT UPDATE/SELECT */
Node *onConflictWhere;
/* RTI of the EXCLUDED pseudo relation */
Index exclRelRTI;
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 2edf04c78f3..9f84e2aa05a 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -267,11 +267,28 @@ insert into insertconflicttest values (1, 'Apple') on conflict (key) do select r
1 | Apple
(1 row)
-insert into insertconflicttest values (1, 'Apple') on conflict (key) do select where fruit <> 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Orange' returning *;
+ key | fruit
+-----+-------
+(0 rows)
+
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Apple' returning *;
key | fruit
-----+-------
(0 rows)
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Orange' returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
delete from insertconflicttest where fruit = 'Apple';
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
key | fruit
@@ -285,11 +302,28 @@ insert into insertconflicttest values (1, 'Apple') on conflict (key) do select f
1 | Apple
(1 row)
-insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update where fruit <> 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Orange' returning *;
+ key | fruit
+-----+-------
+(0 rows)
+
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Apple' returning *;
key | fruit
-----+-------
(0 rows)
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Orange' returning *;
+ key | fruit
+-----+-------
+ 1 | Apple
+(1 row)
+
insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*;
key | fruit | key | fruit | key | fruit
-----+-------+-----+-------+-----+-------
@@ -299,7 +333,7 @@ insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do se
insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*;
key | fruit | key | fruit | key | fruit
-----+-------+-----+-------+-----+-------
- 3 | Pear | | | 3 | Pear
+ 3 | Pear | 3 | Pear | 3 | Pear
(1 row)
explain (costs off) insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for key share returning *;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 47478969135..0e49eda2551 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3507,6 +3507,61 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
(3 rows)
DROP RULE hat_upsert ON hats;
+-- DO SELECT with a WHERE clause
+CREATE RULE hat_confsel AS ON INSERT TO hats
+ DO INSTEAD
+ INSERT INTO hat_data VALUES (
+ NEW.hat_name,
+ NEW.hat_color)
+ ON CONFLICT (hat_name)
+ DO SELECT FOR UPDATE
+ WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.*
+ RETURNING *;
+SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename;
+ definition
+--------------------------------------------------------------------------------------
+ CREATE RULE hat_confsel AS +
+ ON INSERT TO public.hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) +
+ VALUES (new.hat_name, new.hat_color) ON CONFLICT(hat_name) DO SELECT FOR UPDATE +
+ WHERE ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*))+
+ RETURNING hat_data.hat_name, +
+ hat_data.hat_color;
+(1 row)
+
+-- fails without RETURNING
+INSERT INTO hats VALUES ('h7', 'blue');
+ERROR: ON CONFLICT DO SELECT requires a RETURNING clause
+DETAIL: A rule action is INSERT ... ON CONFLICT DO SELECT, which requires a RETURNING clause
+-- works (returns conflicts)
+EXPLAIN (costs off)
+INSERT INTO hats VALUES ('h7', 'blue') RETURNING *;
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------
+ Insert on hat_data
+ Conflict Resolution: SELECT FOR UPDATE
+ Conflict Arbiter Indexes: hat_data_unique_idx
+ Conflict Filter: ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*))
+ -> Result
+(5 rows)
+
+INSERT INTO hats VALUES ('h7', 'blue') RETURNING *;
+ hat_name | hat_color
+------------+------------
+ h7 | black
+(1 row)
+
+-- conflicts excluded by WHERE clause
+INSERT INTO hats VALUES ('h7', 'forbidden') RETURNING *;
+ hat_name | hat_color
+----------+-----------
+(0 rows)
+
+INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
+ hat_name | hat_color
+----------+-----------
+(0 rows)
+
+DROP RULE hat_confsel ON hats;
drop table hats;
drop table hat_data;
-- test for pg_get_functiondef properly regurgitating SET parameters
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index b80b7dae91a..72b8147f849 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -106,11 +106,17 @@ delete from insertconflicttest where fruit = 'Apple';
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select; -- fails
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *;
-insert into insertconflicttest values (1, 'Apple') on conflict (key) do select where fruit <> 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Orange' returning *;
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Orange' returning *;
delete from insertconflicttest where fruit = 'Apple';
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *;
-insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update where fruit <> 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Orange' returning *;
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Apple' returning *;
+insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Orange' returning *;
insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*;
insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index fdd3ff1d161..9206a7f8887 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1205,6 +1205,32 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
DROP RULE hat_upsert ON hats;
+-- DO SELECT with a WHERE clause
+CREATE RULE hat_confsel AS ON INSERT TO hats
+ DO INSTEAD
+ INSERT INTO hat_data VALUES (
+ NEW.hat_name,
+ NEW.hat_color)
+ ON CONFLICT (hat_name)
+ DO SELECT FOR UPDATE
+ WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.*
+ RETURNING *;
+SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename;
+
+-- fails without RETURNING
+INSERT INTO hats VALUES ('h7', 'blue');
+
+-- works (returns conflicts)
+EXPLAIN (costs off)
+INSERT INTO hats VALUES ('h7', 'blue') RETURNING *;
+INSERT INTO hats VALUES ('h7', 'blue') RETURNING *;
+
+-- conflicts excluded by WHERE clause
+INSERT INTO hats VALUES ('h7', 'forbidden') RETURNING *;
+INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
+
+DROP RULE hat_confsel ON hats;
+
drop table hats;
drop table hat_data;
--
2.43.0
