From 66b2e18d738f716373832528e9e52711df76f9c3 Mon Sep 17 00:00:00 2001 From: Viktor Holmberg 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 ON CONFLICT DO UPDATE clause, the old values will be non-NULL for conflicting rows. Similarly, in an INSERT with an - ON CONFLICT DO SELECT clause, you can look at the old values to determine if your query inserted a row or not. If a DELETE is turned into an + ON CONFLICT DO SELECT clause, you can look at the old + values to determine if your query inserted a row or not. + If a DELETE is turned into an UPDATE by a rewrite rule, the new values may be non-NULL. 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 table_name [ AS ON CONFLICT DO UPDATE ... WHERE clause condition was not satisfied, the - row will not be returned. ON CONFLICT DO SELECT - works similarly, except no update takes place. + row will not be returned. @@ -130,7 +129,7 @@ INSERT INTO table_name [ AS SELECT privilege on any column whose values are read in the ON CONFLICT DO UPDATE expressions or condition. If using a - WHERE with ON CONFLICT DO UPDATE / SELECT , + WHERE with ON CONFLICT DO UPDATE / SELECT, you must have SELECT privilege on the columns referenced in the WHERE clause. 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