Re: ON CONFLICT DO SELECT (take 3)

On Tue, Nov 25, 2025 at 9:24 PM Viktor Holmberg <v@viktorh.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/