v19-0004-Fixes-after-Jian-s-review-on-the-12th-dec.patch

From 66b2e18d738f716373832528e9e52711df76f9c3 Mon Sep 17 00:00:00 2001
From: Viktor Holmberg <v@viktorh.net>
Date: Tue, 16 Dec 2025 12:13:22 +0100
Subject: [PATCH v19 4/4] Fixes after Jian's review on the 12th dec

- Updates needed after 2bc7e886fc1baaeee3987a141bff3ac490037d12 was merged
- Clarify that tuple is always visible in ExecOnConflictSelect
- minor doc updates
- minor comment updates
---
 doc/src/sgml/dml.sgml                         |  4 +++-
 doc/src/sgml/ref/insert.sgml                  |  5 ++--
 src/backend/executor/execIndexing.c           |  2 +-
 src/backend/executor/nodeModifyTable.c        |  7 +++---
 src/backend/optimizer/plan/setrefs.c          |  2 +-
 src/backend/optimizer/util/plancat.c          |  8 ++++---
 src/test/regress/expected/insert_conflict.out | 13 +++++-----
 src/test/regress/expected/triggers.out        | 11 +++++++--
 src/test/regress/expected/updatable_views.out | 19 +++++++++++----
 src/test/regress/sql/insert_conflict.sql      |  3 ++-
 src/test/regress/sql/triggers.sql             |  5 ++--
 src/test/regress/sql/updatable_views.sql      | 24 +++++++++++++++----
 12 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml
index 7e5cce0bff0..988e19b347b 100644
--- a/doc/src/sgml/dml.sgml
+++ b/doc/src/sgml/dml.sgml
@@ -388,7 +388,9 @@ UPDATE products SET price = price * 1.10
    <link linkend="sql-on-conflict"><literal>ON CONFLICT DO UPDATE</literal></link>
    clause, the old values will be non-<literal>NULL</literal> for conflicting
    rows.  Similarly, in an <command>INSERT</command> with an
-   <literal>ON CONFLICT DO SELECT</literal> clause, you can look at the old values to determine if your query inserted a row or not. If a <command>DELETE</command> is turned into an
+   <literal>ON CONFLICT DO SELECT</literal> clause, you can look at the old 
+   values to determine if your query inserted a row or not. 
+   If a <command>DELETE</command> is turned into an
    <command>UPDATE</command> by a <link linkend="sql-createrule">rewrite rule</link>,
    the new values may be non-<literal>NULL</literal>.
   </para>
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index f6122eeb12e..ec0eb11ceb0 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -105,8 +105,7 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac
    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.  <literal>ON CONFLICT DO SELECT</literal>
-   works similarly, except no update takes place.
+   row will not be returned.
   </para>
 
   <para>
@@ -130,7 +129,7 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac
    also requires <literal>SELECT</literal> privilege on any column whose
    values are read in the <literal>ON CONFLICT DO UPDATE</literal>
    expressions or <replaceable>condition</replaceable>. If using a
-   <literal>WHERE</literal> with <literal>ON CONFLICT DO UPDATE / SELECT </literal>,
+   <literal>WHERE</literal> with <literal>ON CONFLICT DO UPDATE / SELECT</literal>,
    you must have <literal>SELECT</literal> privilege on the columns referenced
    in the <literal>WHERE</literal> clause.
   </para>
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 0b3a31f1703..7ba552db917 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -54,7 +54,7 @@
  * ---------------------
  *
  * Speculative insertion is a two-phase mechanism used to implement
- * INSERT ... ON CONFLICT DO UPDATE/NOTHING.  The tuple is first inserted
+ * INSERT ... ON CONFLICT DO UPDATE/SELECT/NOTHING.  The tuple is first inserted
  * to the heap and update the indexes as usual, but if a constraint is
  * violated, we can still back out the insertion without aborting the whole
  * transaction.  In an INSERT ... ON CONFLICT statement, if a conflict is
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index c74ceda4a78..0fe6d4a5ac1 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3025,9 +3025,8 @@ ExecOnConflictSelect(ModifyTableContext *context,
 
 	if (lockStrength == LCS_NONE)
 	{
-		if (!table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing))
-			/* The pre-existing tuple was deleted */
-			return false;
+		/* Evem if the tuple is deleted, it must still be physically present */
+		Assert(table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing));
 	}
 	else
 	{
@@ -3048,7 +3047,7 @@ ExecOnConflictSelect(ModifyTableContext *context,
 				lockmode = LockTupleExclusive;
 				break;
 			default:
-				elog(ERROR, "unexpected lock strength %d", lockStrength);
+				elog(ERROR, "Unexpected lock strength %d", lockStrength);
 		}
 
 		if (!ExecOnConflictLockRow(context, existing, conflictTid,
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index f54cd93949a..412cba9af6c 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -3097,7 +3097,7 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
  *	  other-relation Vars by OUTER_VAR references, while leaving target Vars
  *	  alone. Thus inner_itlist = NULL and acceptable_rel = the ID of the
  *	  target relation should be passed.
- * 3) ON CONFLICT UPDATE SET/WHERE clauses.  Here references to EXCLUDED are
+ * 3) ON CONFLICT UPDATE SET, SELECT & WHERE clauses.  Here references to EXCLUDED are
  *	  to be replaced with INNER_VAR references, while leaving target Vars (the
  *	  to-be-updated relation) alone. Correspondingly inner_itlist is to be
  *	  EXCLUDED elements, outer_itlist = NULL and acceptable_rel the target
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index a19f995a03b..f92ff559441 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1042,10 +1042,12 @@ infer_arbiter_indexes(PlannerInfo *root)
 		else if (indexOidFromConstraint != InvalidOid)
 		{
 			/*
-			 * In the case of "ON constraint_name DO UPDATE" we need to skip
-			 * non-unique candidates.
+			 * In the case of "ON constraint_name DO UPDATE/SELECT" we need to
+			 * skip non-unique candidates.
 			 */
-			if (!idxForm->indisunique && onconflict->action == ONCONFLICT_UPDATE)
+			if (!idxForm->indisunique &&
+				(onconflict->action == ONCONFLICT_UPDATE ||
+				 onconflict->action == ONCONFLICT_SELECT))
 				continue;
 		}
 		else
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index ea81b6208e7..03f92ccd670 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -1093,12 +1093,13 @@ select * from parted_conflict_test order by a;
 -- test DO SELECT with multiple rows hitting different partitions
 truncate parted_conflict_test;
 insert into parted_conflict_test (a, b) values (1, 'a'), (2, 'b'), (4, 'c');
-insert into parted_conflict_test (a, b) values (1, 'x'), (2, 'y'), (4, 'z') on conflict (a) do select returning *;
- a | b 
----+---
- 1 | a
- 2 | b
- 4 | c
+insert into parted_conflict_test (a, b) values (1, 'x'), (2, 'y'), (4, 'z')
+  on conflict (a) do select returning *, tableoid::regclass;
+ a | b |        tableoid        
+---+---+------------------------
+ 1 | a | parted_conflict_test_1
+ 2 | b | parted_conflict_test_1
+ 4 | c | parted_conflict_test_3
 (3 rows)
 
 -- should see original values (1, 'a'), (2, 'b'), (4, 'c')
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 98e56ecaef8..21839f5d8c6 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1669,8 +1669,8 @@ PL/pgSQL function trigger_ddl_func() line 3 at SQL statement
 drop table trigger_ddl_table;
 drop function trigger_ddl_func();
 --
--- Verify behavior of before and after triggers with INSERT...ON CONFLICT
--- DO UPDATE
+-- Verify behavior of before and after triggers with 
+-- INSERT...ON CONFLICT DO UPDATE / SELECT
 --
 create table upsert (key int4 primary key, color text);
 create function upsert_before_func()
@@ -1745,6 +1745,13 @@ insert into upsert values(8, 'yellow') on conflict (key) do update set color = '
 WARNING:  before insert (new): (8,yellow)
 WARNING:  before insert (new, modified): (9,"yellow trig modified")
 WARNING:  after insert (new): (9,"yellow trig modified")
+insert into upsert values(9, 'orange') on conflict (key) do select for update returning old.*, new.*, upsert.*;
+WARNING:  before insert (new): (9,orange)
+ key |        color         | key |        color         | key |        color         
+-----+----------------------+-----+----------------------+-----+----------------------
+   9 | yellow trig modified |   9 | yellow trig modified |   9 | yellow trig modified
+(1 row)
+
 insert into upsert values(3, 'orange') on conflict (key) do select for update returning old.*, new.*, upsert.*;
 WARNING:  before insert (new): (3,orange)
  key |           color           | key |           color           | key |           color           
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index a3c811effc8..4aa9c29ce2b 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -319,30 +319,39 @@ SELECT * FROM rw_view15;
 -- Test ON CONFLICT DO SELECT with updatable views
 -- This tests behavior consistency between DO SELECT and DO UPDATE when using WHERE clauses
 -- Note: rw_view15 is defined as "SELECT a, upper(b) FROM base_tbl" where base_tbl.b has DEFAULT 'Unspecified'
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT RETURNING *; -- needs RETURNING, should return existing row
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT RETURNING *; -- needs RETURNING, should return existing row
  a |    upper    
 ---+-------------
  3 | UNSPECIFIED
 (1 row)
 
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; -- WHERE on view column (uppercase)
+-- WHERE on view column (uppercase)
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; 
  a |    upper    
 ---+-------------
  3 | UNSPECIFIED
 (1 row)
 
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE SET a = excluded.a WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; -- compare DO UPDATE with same WHERE
+-- compare DO UPDATE with same WHERE
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO UPDATE SET a = excluded.a WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; 
  a |    upper    
 ---+-------------
  3 | UNSPECIFIED
 (1 row)
 
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT WHERE excluded.upper = 'Unspecified' RETURNING *; -- WHERE on excluded value (mixed case)
+-- WHERE on excluded value (mixed case)
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT WHERE excluded.upper = 'Unspecified' RETURNING *; 
  a | upper 
 ---+-------
 (0 rows)
 
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE SET a = excluded.a WHERE excluded.upper = 'Unspecified' RETURNING *; -- compare DO UPDATE with same WHERE
+-- compare DO UPDATE with same WHERE
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO UPDATE SET a = excluded.a WHERE excluded.upper = 'Unspecified' RETURNING *; 
  a | upper 
 ---+-------
 (0 rows)
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index e746c011b5e..a3ca49372d2 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -621,7 +621,8 @@ select * from parted_conflict_test order by a;
 -- test DO SELECT with multiple rows hitting different partitions
 truncate parted_conflict_test;
 insert into parted_conflict_test (a, b) values (1, 'a'), (2, 'b'), (4, 'c');
-insert into parted_conflict_test (a, b) values (1, 'x'), (2, 'y'), (4, 'z') on conflict (a) do select returning *;
+insert into parted_conflict_test (a, b) values (1, 'x'), (2, 'y'), (4, 'z')
+  on conflict (a) do select returning *, tableoid::regclass;
 
 -- should see original values (1, 'a'), (2, 'b'), (4, 'c')
 select * from parted_conflict_test order by a;
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index ee451ec7ed3..ae1b46fa799 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1147,8 +1147,8 @@ drop table trigger_ddl_table;
 drop function trigger_ddl_func();
 
 --
--- Verify behavior of before and after triggers with INSERT...ON CONFLICT
--- DO UPDATE
+-- Verify behavior of before and after triggers with 
+-- INSERT...ON CONFLICT DO UPDATE / SELECT
 --
 create table upsert (key int4 primary key, color text);
 
@@ -1197,6 +1197,7 @@ insert into upsert values(5, 'purple') on conflict (key) do update set color = '
 insert into upsert values(6, 'white') on conflict (key) do update set color = 'updated ' || upsert.color;
 insert into upsert values(7, 'pink') on conflict (key) do update set color = 'updated ' || upsert.color;
 insert into upsert values(8, 'yellow') on conflict (key) do update set color = 'updated ' || upsert.color;
+insert into upsert values(9, 'orange') on conflict (key) do select for update returning old.*, new.*, upsert.*;
 insert into upsert values(3, 'orange') on conflict (key) do select for update returning old.*, new.*, upsert.*;
 insert into upsert values(3, 'orange') on conflict (key) do select for update where upsert.key = 4 returning old.*, new.*, upsert.*;
 
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index d9f1ca5bd97..323f661b3ec 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -109,11 +109,25 @@ SELECT * FROM rw_view15;
 -- Test ON CONFLICT DO SELECT with updatable views
 -- This tests behavior consistency between DO SELECT and DO UPDATE when using WHERE clauses
 -- Note: rw_view15 is defined as "SELECT a, upper(b) FROM base_tbl" where base_tbl.b has DEFAULT 'Unspecified'
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT RETURNING *; -- needs RETURNING, should return existing row
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; -- WHERE on view column (uppercase)
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE SET a = excluded.a WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; -- compare DO UPDATE with same WHERE
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO SELECT WHERE excluded.upper = 'Unspecified' RETURNING *; -- WHERE on excluded value (mixed case)
-INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE SET a = excluded.a WHERE excluded.upper = 'Unspecified' RETURNING *; -- compare DO UPDATE with same WHERE
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT RETURNING *; -- needs RETURNING, should return existing row
+
+-- WHERE on view column (uppercase)
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; 
+
+-- compare DO UPDATE with same WHERE
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO UPDATE SET a = excluded.a WHERE excluded.upper = 'UNSPECIFIED' RETURNING *; 
+
+-- WHERE on excluded value (mixed case)
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO SELECT WHERE excluded.upper = 'Unspecified' RETURNING *; 
+
+-- compare DO UPDATE with same WHERE
+INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a)
+DO UPDATE SET a = excluded.a WHERE excluded.upper = 'Unspecified' RETURNING *; 
+
 SELECT * FROM rw_view15;
 ALTER VIEW rw_view15 ALTER COLUMN upper SET DEFAULT 'NOT SET';
 INSERT INTO rw_view15 (a) VALUES (4); -- should fail
-- 
2.48.1