Re: ON CONFLICT DO SELECT (take 3)

On Tue, Nov 25, 2025 at 9:24 PM Viktor Holmberg <v(at)viktorh(dot)net> wrote:
>
> In conclusion:
> Attached is v17, with:
> - Jians latest patches minus the injection point testing
> - Doc for MVCC
> - ExecOnConflictSelect with a default clause for lockStrength.
>

hi.

+ <para>
+ Insert a new distributor if the name doesn't match, otherwise return
+ the existing row. This example uses the <varname>excluded</varname>
+ table in the WHERE clause to filter results:
+<programlisting>
+INSERT INTO distributors (did, dname) VALUES (12, 'Micro Devices Inc')
+ ON CONFLICT (did) DO SELECT WHERE dname = EXCLUDED.dname
+ RETURNING *;
+</programlisting>
+ </para>

"ON CONFLICT (did)":
"Insert a new distributor if the name doesn't match",
i think it should be
"Insert a new distributor if the distributor id doesn't match",
suppose "did" refer to distributor id.

/*
- * If there is a WHERE clause, initialize state where it will
- * be evaluated, mapping the attribute numbers appropriately.
- * As with onConflictSet, we need to map partition varattnos
- * to the partition's tupdesc.
+ * For both ON CONFLICT DO UPDATE and ON CONFLICT DO SELECT,
+ * there may be a WHERE clause. If so, initialize state where
+ * it will be evaluated, mapping the attribute numbers
+ * appropriately. As with onConflictSet, we need to map
+ * partition varattnos twice, to catch both the EXCLUDED
+ * pseudo-relation (INNER_VAR), and the main target relation
+ * (firstVarno).
*/
if (node->onConflictWhere)
{
List *clause;

+ if (part_attmap == NULL)
+ part_attmap =
+ build_attrmap_by_name(RelationGetDescr(partrel),
+ RelationGetDescr(firstResultRel),
+ false);
+
we already processed onConflictSet. the above comments need change?

heap_lock_tuple comments:
/*
* This is possible, but only when locking a tuple for ON CONFLICT
* UPDATE. We return this value here rather than throwing an error in
* order to give that case the opportunity to throw a more specific
* error.
*/
+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;

the above tests showing TM_Invisible is possible for ON CONFLICT DO SELECT.
so the above heap_lock_tuple comments also need change.

+--
+-- INSERT ... ON CONFLICT DO SELECT and Row-level security
+--
+
+SET SESSION AUTHORIZATION regress_rls_alice;
+DROP POLICY p3_with_all ON document;
+
+CREATE POLICY p1_select_novels ON document FOR SELECT
+ USING (cid = (SELECT cid from category WHERE cname = 'novel'));
+CREATE POLICY p2_insert_own ON document FOR INSERT
+ WITH CHECK (dauthor = current_user);
+CREATE POLICY p3_update_novels ON document FOR UPDATE
+ USING (cid = (SELECT cid from category WHERE cname = 'novel'))
+ WITH CHECK (dauthor = current_user);
+
+SET SESSION AUTHORIZATION regress_rls_bob;

create_policy.sgml "Policies Applied by Command Type" distinguish ON
CONFLICT SELECT FOR UPDATE
and ON CONFLICT SELECT is that update will invoke the UPDATE USING policy.

The above tests p1_select_novels, p3_update_novels have the same using part.
SELECT FOR UPDATE will fail just like the same reason as ON CONFLICT SELECT
so I think the above tests do not fully test the SELECT FOR UPDATE scarenio.
please check the attached file, which slightly changed
p3_update_novels USING qual.

one minor issue, ruleutils.c: get_lock_clause_strength
I think it make more sense to remove the prefix whitespace, like change
``return " FOR KEY SHARE";``
to
``return "FOR KEY SHARE";``
and let caller add the whitespace itself.

--
jian
https://www.enterprisedb.com/