v8-0002-Review-comments-for-ON-CONFLICT-DO-SELECT.patch
text/x-patch
Filename: v8-0002-Review-comments-for-ON-CONFLICT-DO-SELECT.patch
Type: text/x-patch
Part: 1
Message:
Re: ON CONFLICT DO SELECT (take 3)
From b40a61fb94e11962801e1e51f084a6a18d02f524 Mon Sep 17 00:00:00 2001
From: Dean Rasheed <dean.a.rasheed@gmail.com>
Date: Mon, 31 Mar 2025 15:20:47 +0100
Subject: [PATCH v8 2/3] 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 9f1b90a0eb2..1a575cc96e8 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4670,7 +4670,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";
@@ -4678,6 +4678,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
resolution = "UPDATE";
else
{
+ Assert(node->onConflictAction == ONCONFLICT_SELECT);
switch (node->onConflictLockingStrength)
{
case LCS_NONE:
@@ -4692,9 +4693,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;
}
}
@@ -4707,7 +4712,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 6b9f17366bd..80e2650366c 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -161,6 +161,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,
@@ -1175,13 +1176,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);
@@ -2731,6 +2732,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,
@@ -2888,7 +2895,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;
@@ -2933,11 +2940,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
@@ -2985,13 +2993,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)
{
@@ -3049,11 +3062,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))
@@ -3066,19 +3081,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,
@@ -3088,8 +3099,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++;
@@ -3106,6 +3118,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 ccdc9bc264a..b4d9a998e07 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1140,7 +1140,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 af50d705091..a41516ee962 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1028,11 +1028,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)
@@ -1192,12 +1195,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;
@@ -1226,27 +1230,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,
@@ -1260,13 +1265,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 adc9e7600e1..f3cd32b7222 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 556ab057e5a..82e467a0b2f 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);
@@ -5997,30 +5998,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,
@@ -6033,6 +6013,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.
@@ -7125,7 +7127,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 */
@@ -7140,6 +7142,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 43c4b62838e..bdbbebd49fd 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -368,7 +368,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 f3d2e1da802..d226c472340 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 7c52181cbcb..be5d59b08ca 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3562,6 +3562,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 3f240bec7b0..40f5c16e540 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.51.0