From 6ebf246af87f1515c86762bbdf79fc40be94068b Mon Sep 17 00:00:00 2001 From: Viktor Holmberg Date: Thu, 20 Nov 2025 20:18:00 +0100 Subject: [PATCH v14 4/4] ON CONFLCIT DO SELECT: More review fixes - Docs updated in a bunch of forgotten places - Test diff made smaller against master - ExecOnConflictSelect minor refactor: - Invert if statment to avoid negation - Add comment for the LCS_NONE case - Remove the switch default case, make compiler check all possible cases - Improve some comments - Give CMD_INSERT to ExecProcessReturning --- doc/src/sgml/fdwhandler.sgml | 2 +- doc/src/sgml/postgres-fdw.sgml | 2 +- doc/src/sgml/ref/create_policy.sgml | 4 +- doc/src/sgml/ref/create_table.sgml | 2 +- doc/src/sgml/ref/insert.sgml | 19 ++++---- doc/src/sgml/ref/merge.sgml | 3 +- src/backend/executor/nodeModifyTable.c | 28 ++++++------ src/test/regress/expected/insert_conflict.out | 44 ++++++++++--------- src/test/regress/sql/insert_conflict.sql | 43 +++++++++--------- 9 files changed, 79 insertions(+), 68 deletions(-) diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index c6d66414b8e..f6d4e51efd8 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -2045,7 +2045,7 @@ GetForeignServerByName(const char *name, bool missing_ok); INSERT with an ON CONFLICT clause does not support specifying the conflict target, as unique constraints or exclusion constraints on remote tables are not locally known. This - in turn implies that ON CONFLICT DO UPDATE is not supported, + in turn implies that ON CONFLICT DO UPDATE / SELECT is not supported, since the specification is mandatory there. diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 781a01067f7..570323e09ce 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -82,7 +82,7 @@ Note that postgres_fdw currently lacks support for INSERT statements with an ON CONFLICT DO - UPDATE clause. However, the ON CONFLICT DO NOTHING + UPDATE / SELECT clause. However, the ON CONFLICT DO NOTHING clause is supported, provided a unique index inference specification is omitted. Note also that postgres_fdw supports row movement diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml index 09fd26f7b7d..e798eacfb42 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -574,7 +574,7 @@ CREATE POLICY name ON ON CONFLICT DO SELECT - Check existing rows + Check existing row — — — @@ -582,7 +582,7 @@ CREATE POLICY name ON ON CONFLICT DO SELECT FOR UPDATE/SHARE - Check existing rows + Check existing row — Existing row — diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 6557c5cffd8..bcafe2458f9 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -1380,7 +1380,7 @@ WITH ( MODULUS numeric_literal, REM clause. NOT NULL and CHECK constraints are not deferrable. Note that deferrable constraints cannot be used as conflict arbitrators in an INSERT statement that - includes an ON CONFLICT DO UPDATE clause. + includes an ON CONFLICT DO UPDATE / SELECT clause. diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index 7b883b799b5..5054bd73261 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -115,6 +115,10 @@ INSERT INTO table_name [ AS UPDATE privilege on the table is also required. If ON CONFLICT DO SELECT is present, SELECT privilege on the table is required. + If ON CONFLICT DO SELECT is used with + FOR UPDATE or FOR SHARE, + UPDATE privilege is also required on at least one + column, in addition to SELECT privilege. @@ -122,13 +126,13 @@ INSERT INTO table_name [ AS INSERT privilege on the listed columns. Similarly, when ON CONFLICT DO UPDATE is specified, you only need UPDATE privilege on the column(s) that are - listed to be updated. However, ON CONFLICT DO UPDATE + listed to be updated. However, ON CONFLICT DO UPDATE also requires SELECT privilege on any column whose values are read in the ON CONFLICT DO UPDATE - expressions or condition. - For ON CONFLICT DO SELECT, SELECT - privilege is required on any column whose values are read in the - condition. + expressions or condition. If using a + WHERE with ON CONFLICT DO UPDATE / SELECT , + you must have SELECT privilege on the columns referenced + in the WHERE clause. @@ -599,7 +603,7 @@ INSERT INTO table_name [ AS Note that exclusion constraints are not supported as arbiters with - ON CONFLICT DO UPDATE. In all cases, only + ON CONFLICT DO UPDATE / SELECT. In all cases, only NOT DEFERRABLE constraints and unique indexes are supported as arbiters. @@ -667,8 +671,7 @@ INSERT oid countINSERT command contains a RETURNING clause, the result will be similar to that of a SELECT statement containing the columns and values defined in the - RETURNING list, computed over the row(s) inserted or - updated by the command. + RETURNING list, computed over the row(s) affected by the command. diff --git a/doc/src/sgml/ref/merge.sgml b/doc/src/sgml/ref/merge.sgml index c2e181066a4..765fe7a7d62 100644 --- a/doc/src/sgml/ref/merge.sgml +++ b/doc/src/sgml/ref/merge.sgml @@ -714,7 +714,8 @@ MERGE total_count on the behavior at each isolation level. You may also wish to consider using INSERT ... ON CONFLICT as an alternative statement which offers the ability to run an - UPDATE if a concurrent INSERT + UPDATE or return the existing row (with + DO SELECT) if a concurrent INSERT occurs. There are a variety of differences and restrictions between the two statement types and they are not interchangeable. diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 51d0d0a217c..d4361a7527f 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -3023,8 +3023,13 @@ ExecOnConflictSelect(ModifyTableContext *context, */ Assert(!resultRelInfo->ri_needLockTagTuple); - /* Lock or fetch the existing tuple to select */ - if (lockStrength != LCS_NONE) + if (lockStrength == LCS_NONE) + { + if (!table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing)) + /* The pre-existing tuple was deleted */ + return false; + } + else { LockTupleMode lockmode; @@ -3042,20 +3047,16 @@ ExecOnConflictSelect(ModifyTableContext *context, case LCS_FORUPDATE: lockmode = LockTupleExclusive; break; - default: - elog(ERROR, "unexpected lock strength %d", lockStrength); + case LCS_NONE: + /* Prevent a compiler warning */ + pg_unreachable(); } if (!ExecOnConflictLockRow(context, existing, conflictTid, resultRelInfo->ri_RelationDesc, lockmode, false)) return false; } - else - { - if (!table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing)) - return false; - } - + /* * For the same reasons as ExecOnConflictUpdate, we must verify that the * tuple is visible to our snapshot. @@ -3097,11 +3098,12 @@ ExecOnConflictSelect(ModifyTableContext *context, mtstate->ps.state); } - /* Parse analysis should already have disallowed this */ + /* Parse analysis should already have disallowed this, as RETURNING + * is required for DO SELECT. + */ Assert(resultRelInfo->ri_projectReturning); - /* Process RETURNING like an UPDATE that didn't change anything */ - *rslot = ExecProcessReturning(context, resultRelInfo, CMD_UPDATE, + *rslot = ExecProcessReturning(context, resultRelInfo, CMD_INSERT, existing, existing, context->planSlot); if (canSetTag) diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 92d2f38aa08..839e33883a0 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -410,6 +410,8 @@ explain (costs off) insert into insertconflicttest values (1, 'Apple') on confli -> Result (4 rows) +truncate insertconflicttest; +insert into insertconflicttest values (1, 'Apple'), (2, 'Orange'); -- Give good diagnostic message when EXCLUDED.* spuriously referenced from -- RETURNING: insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit; @@ -430,26 +432,26 @@ LINE 1: ... 'Apple') on conflict (key) do update set fruit = excluded.f... ^ HINT: Perhaps you meant to reference the column "excluded.fruit". -- inference fails: -insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (5, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (4, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (6, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (5, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (7, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (6, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -- Check the target relation can be aliased -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name ERROR: invalid reference to FROM-clause entry for table "insertconflicttest" LINE 1: ...onfruit') on conflict (key) do update set fruit = insertconf... ^ HINT: Perhaps you meant to reference the table alias "ict". -- Check helpful hint when qualifying set column with target table -insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango'; +insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango'; ERROR: column "insertconflicttest" of relation "insertconflicttest" does not exist -LINE 1: ...4, 'Kiwi') on conflict (key, fruit) do update set insertconf... +LINE 1: ...3, 'Kiwi') on conflict (key, fruit) do update set insertconf... ^ HINT: SET target columns cannot be qualified with the relation name. drop index key_index; @@ -458,16 +460,16 @@ drop index key_index; -- create unique index comp_key_index on insertconflicttest(key, fruit); -- inference succeeds: -insert into insertconflicttest values (8, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit; -insert into insertconflicttest values (9, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (7, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (8, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit; -- inference fails: -insert into insertconflicttest values (10, 'Banana') on conflict (key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (9, 'Banana') on conflict (key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (11, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (10, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (12, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (11, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (13, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (12, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification drop index comp_key_index; -- @@ -476,17 +478,17 @@ drop index comp_key_index; create unique index part_comp_key_index on insertconflicttest(key, fruit) where key < 5; create unique index expr_part_comp_key_index on insertconflicttest(key, lower(fruit)) where key < 5; -- inference fails: -insert into insertconflicttest values (14, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (13, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (15, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (14, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (16, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (15, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (17, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (16, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (18, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (17, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -insert into insertconflicttest values (19, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (18, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification drop index part_comp_key_index; drop index expr_part_comp_key_index; diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index 495c193a763..8f856cdb245 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -157,6 +157,9 @@ insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do explain (costs off) insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for key share returning *; +truncate insertconflicttest; +insert into insertconflicttest values (1, 'Apple'), (2, 'Orange'); + -- Give good diagnostic message when EXCLUDED.* spuriously referenced from -- RETURNING: insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit; @@ -168,18 +171,18 @@ insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do update insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruitt; -- inference fails: -insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit; -insert into insertconflicttest values (5, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (6, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit; -insert into insertconflicttest values (7, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (4, 'Mango') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (5, 'Lemon') on conflict (fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (6, 'Passionfruit') on conflict (lower(fruit)) do update set fruit = excluded.fruit; -- Check the target relation can be aliased -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias -insert into insertconflicttest AS ict values (7, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = excluded.fruit; -- ok, no reference to target table +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = ict.fruit; -- ok, alias +insert into insertconflicttest AS ict values (6, 'Passionfruit') on conflict (key) do update set fruit = insertconflicttest.fruit; -- error, references aliased away name -- Check helpful hint when qualifying set column with target table -insert into insertconflicttest values (4, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango'; +insert into insertconflicttest values (3, 'Kiwi') on conflict (key, fruit) do update set insertconflicttest.fruit = 'Mango'; drop index key_index; @@ -189,14 +192,14 @@ drop index key_index; create unique index comp_key_index on insertconflicttest(key, fruit); -- inference succeeds: -insert into insertconflicttest values (8, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit; -insert into insertconflicttest values (9, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (7, 'Raspberry') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (8, 'Lime') on conflict (fruit, key) do update set fruit = excluded.fruit; -- inference fails: -insert into insertconflicttest values (10, 'Banana') on conflict (key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (11, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (12, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; -insert into insertconflicttest values (13, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (9, 'Banana') on conflict (key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (10, 'Blueberry') on conflict (key, key, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (11, 'Cherry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (12, 'Date') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; drop index comp_key_index; @@ -207,12 +210,12 @@ create unique index part_comp_key_index on insertconflicttest(key, fruit) where create unique index expr_part_comp_key_index on insertconflicttest(key, lower(fruit)) where key < 5; -- inference fails: -insert into insertconflicttest values (14, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit; -insert into insertconflicttest values (15, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (16, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (17, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit; -insert into insertconflicttest values (18, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; -insert into insertconflicttest values (19, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (13, 'Grape') on conflict (key, fruit) do update set fruit = excluded.fruit; +insert into insertconflicttest values (14, 'Raisin') on conflict (fruit, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (15, 'Cranberry') on conflict (key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (16, 'Melon') on conflict (key, key, key) do update set fruit = excluded.fruit; +insert into insertconflicttest values (17, 'Mulberry') on conflict (key, lower(fruit)) do update set fruit = excluded.fruit; +insert into insertconflicttest values (18, 'Pineapple') on conflict (lower(fruit), key) do update set fruit = excluded.fruit; drop index part_comp_key_index; drop index expr_part_comp_key_index; -- 2.48.1