From d667db37e6f46b5152c13256690fc6b3fe72fb30 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 25 Nov 2025 14:53:51 +0800 Subject: [PATCH v17 1/3] ON CONFLICT DO SELECT rebase v15 and combine v15-0001-Add-support-for-INSERT-.-ON-CONFLICT-DO-SELECT.patch v15-0002-More-suggested-review-comments.patch v15-0003-extra-tests-for-ONCONFLICT_SELECT-ExecInitPartit.patch v15-0004-ON-CONFLCIT-DO-SELECT-More-review-fixes.patch together. based on https://postgr.es/m/9284d41a-57a6-4a37-ac9f-873cb5c509d4@Spark --- contrib/pgrowlocks/Makefile | 2 +- .../expected/on-conflict-do-select.out | 80 +++++ contrib/pgrowlocks/meson.build | 1 + .../specs/on-conflict-do-select.spec | 39 ++ contrib/postgres_fdw/postgres_fdw.c | 2 +- doc/src/sgml/dml.sgml | 3 +- doc/src/sgml/fdwhandler.sgml | 2 +- doc/src/sgml/postgres-fdw.sgml | 2 +- doc/src/sgml/ref/create_policy.sgml | 16 + doc/src/sgml/ref/insert.sgml | 117 ++++-- doc/src/sgml/ref/merge.sgml | 3 +- src/backend/commands/explain.c | 36 +- src/backend/executor/execPartition.c | 134 ++++--- src/backend/executor/nodeModifyTable.c | 332 ++++++++++++++---- src/backend/optimizer/plan/createplan.c | 6 +- src/backend/optimizer/plan/setrefs.c | 3 +- src/backend/optimizer/util/plancat.c | 14 +- src/backend/parser/analyze.c | 68 ++-- src/backend/parser/gram.y | 20 +- src/backend/parser/parse_clause.c | 14 +- src/backend/rewrite/rewriteHandler.c | 27 +- src/backend/rewrite/rowsecurity.c | 101 +++--- src/backend/utils/adt/ruleutils.c | 69 ++-- src/include/nodes/execnodes.h | 13 +- src/include/nodes/lockoptions.h | 3 +- src/include/nodes/nodes.h | 1 + src/include/nodes/parsenodes.h | 7 +- src/include/nodes/plannodes.h | 4 +- src/include/nodes/primnodes.h | 16 +- .../expected/insert-conflict-do-select.out | 138 ++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/insert-conflict-do-select.spec | 53 +++ src/test/regress/expected/constraints.out | 8 +- src/test/regress/expected/insert_conflict.out | 315 ++++++++++++++++- src/test/regress/expected/rowsecurity.out | 92 ++++- src/test/regress/expected/rules.out | 55 +++ src/test/regress/expected/updatable_views.out | 31 ++ src/test/regress/sql/constraints.sql | 7 +- src/test/regress/sql/insert_conflict.sql | 115 +++++- src/test/regress/sql/rowsecurity.sql | 57 ++- src/test/regress/sql/rules.sql | 26 ++ src/test/regress/sql/updatable_views.sql | 8 + src/tools/pgindent/typedefs.list | 2 +- 43 files changed, 1751 insertions(+), 292 deletions(-) create mode 100644 contrib/pgrowlocks/expected/on-conflict-do-select.out create mode 100644 contrib/pgrowlocks/specs/on-conflict-do-select.spec create mode 100644 src/test/isolation/expected/insert-conflict-do-select.out create mode 100644 src/test/isolation/specs/insert-conflict-do-select.spec diff --git a/contrib/pgrowlocks/Makefile b/contrib/pgrowlocks/Makefile index e8080646643..a1e25b101a9 100644 --- a/contrib/pgrowlocks/Makefile +++ b/contrib/pgrowlocks/Makefile @@ -9,7 +9,7 @@ EXTENSION = pgrowlocks DATA = pgrowlocks--1.2.sql pgrowlocks--1.1--1.2.sql pgrowlocks--1.0--1.1.sql PGFILEDESC = "pgrowlocks - display row locking information" -ISOLATION = pgrowlocks +ISOLATION = pgrowlocks on-conflict-do-select ISOLATION_OPTS = --load-extension=pgrowlocks ifdef USE_PGXS diff --git a/contrib/pgrowlocks/expected/on-conflict-do-select.out b/contrib/pgrowlocks/expected/on-conflict-do-select.out new file mode 100644 index 00000000000..0bafa556844 --- /dev/null +++ b/contrib/pgrowlocks/expected/on-conflict-do-select.out @@ -0,0 +1,80 @@ +Parsed test spec with 2 sessions + +starting permutation: s1_begin s1_doselect_nolock s2_rowlocks s1_rollback +step s1_begin: BEGIN; +step s1_doselect_nolock: INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step s2_rowlocks: SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); +locked_row|multi|modes +----------+-----+----- +(0 rows) + +step s1_rollback: ROLLBACK; + +starting permutation: s1_begin s1_doselect_keyshare s2_rowlocks s1_rollback +step s1_begin: BEGIN; +step s1_doselect_keyshare: INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step s2_rowlocks: SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); +locked_row|multi|modes +----------+-----+----------------- +(0,1) |f |{"For Key Share"} +(1 row) + +step s1_rollback: ROLLBACK; + +starting permutation: s1_begin s1_doselect_share s2_rowlocks s1_rollback +step s1_begin: BEGIN; +step s1_doselect_share: INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR SHARE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step s2_rowlocks: SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); +locked_row|multi|modes +----------+-----+------------- +(0,1) |f |{"For Share"} +(1 row) + +step s1_rollback: ROLLBACK; + +starting permutation: s1_begin s1_doselect_nokeyupd s2_rowlocks s1_rollback +step s1_begin: BEGIN; +step s1_doselect_nokeyupd: INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR NO KEY UPDATE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step s2_rowlocks: SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); +locked_row|multi|modes +----------+-----+--------------------- +(0,1) |f |{"For No Key Update"} +(1 row) + +step s1_rollback: ROLLBACK; + +starting permutation: s1_begin s1_doselect_update s2_rowlocks s1_rollback +step s1_begin: BEGIN; +step s1_doselect_update: INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step s2_rowlocks: SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); +locked_row|multi|modes +----------+-----+-------------- +(0,1) |f |{"For Update"} +(1 row) + +step s1_rollback: ROLLBACK; diff --git a/contrib/pgrowlocks/meson.build b/contrib/pgrowlocks/meson.build index 6007a76ae75..7ebeae55395 100644 --- a/contrib/pgrowlocks/meson.build +++ b/contrib/pgrowlocks/meson.build @@ -31,6 +31,7 @@ tests += { 'isolation': { 'specs': [ 'pgrowlocks', + 'on-conflict-do-select', ], 'regress_args': ['--load-extension=pgrowlocks'], }, diff --git a/contrib/pgrowlocks/specs/on-conflict-do-select.spec b/contrib/pgrowlocks/specs/on-conflict-do-select.spec new file mode 100644 index 00000000000..bbd571f4c21 --- /dev/null +++ b/contrib/pgrowlocks/specs/on-conflict-do-select.spec @@ -0,0 +1,39 @@ +# Tests for ON CONFLICT DO SELECT with row-level locking + +setup +{ + CREATE TABLE conflict_test (key int PRIMARY KEY, val text); + INSERT INTO conflict_test VALUES (1, 'original'); +} + +teardown +{ + DROP TABLE conflict_test; +} + +session s1 +step s1_begin { BEGIN; } +step s1_doselect_nolock { INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT RETURNING *; } +step s1_doselect_keyshare { INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING *; } +step s1_doselect_share { INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR SHARE RETURNING *; } +step s1_doselect_nokeyupd { INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR NO KEY UPDATE RETURNING *; } +step s1_doselect_update { INSERT INTO conflict_test VALUES (1, 'new') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; } +step s1_rollback { ROLLBACK; } + +session s2 +step s2_rowlocks { SELECT locked_row, multi, modes FROM pgrowlocks('conflict_test'); } + +# Test 1: No locking - should not show in pgrowlocks +permutation s1_begin s1_doselect_nolock s2_rowlocks s1_rollback + +# Test 2: FOR KEY SHARE - should show lock +permutation s1_begin s1_doselect_keyshare s2_rowlocks s1_rollback + +# Test 3: FOR SHARE - should show lock +permutation s1_begin s1_doselect_share s2_rowlocks s1_rollback + +# Test 4: FOR NO KEY UPDATE - should show lock +permutation s1_begin s1_doselect_nokeyupd s2_rowlocks s1_rollback + +# Test 5: FOR UPDATE - should show lock +permutation s1_begin s1_doselect_update s2_rowlocks s1_rollback diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 06b52c65300..b793669d97f 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1856,7 +1856,7 @@ postgresPlanForeignModify(PlannerInfo *root, returningList = (List *) list_nth(plan->returningLists, subplan_index); /* - * ON CONFLICT DO UPDATE and DO NOTHING case with inference specification + * ON CONFLICT DO NOTHING/UPDATE/SELECT with inference specification * should have already been rejected in the optimizer, as presently there * is no way to recognize an arbiter index on a foreign table. Only DO * NOTHING is supported without an inference specification. diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml index 61c64cf6c49..7e5cce0bff0 100644 --- a/doc/src/sgml/dml.sgml +++ b/doc/src/sgml/dml.sgml @@ -387,7 +387,8 @@ UPDATE products SET price = price * 1.10 INSERT with an ON CONFLICT DO UPDATE clause, the old values will be non-NULL for conflicting - rows. Similarly, if a DELETE is turned into an + 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 UPDATE by a rewrite rule, the new values may be non-NULL. 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 9b032fbf675..3f00d2fa457 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 42d43ad7bf4..e798eacfb42 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -571,6 +571,22 @@ CREATE POLICY name ON — + + + ON CONFLICT DO SELECT + Check existing row + — + — + — + — + + + ON CONFLICT DO SELECT FOR UPDATE/SHARE + Check existing row + — + Existing row + — + — MERGE diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index 0598b8dea34..4c20560c08b 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -37,6 +37,7 @@ INSERT INTO table_name [ AS and conflict_action is one of: DO NOTHING + DO SELECT [ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } ] [ WHERE condition ] DO UPDATE SET { column_name = { expression | DEFAULT } | ( column_name [, ...] ) = [ ROW ] ( { expression | DEFAULT } [, ...] ) | ( column_name [, ...] ) = ( sub-SELECT ) @@ -88,25 +89,36 @@ INSERT INTO table_name [ AS The optional RETURNING clause causes INSERT - to compute and return value(s) based on each row actually inserted - (or updated, if an ON CONFLICT DO UPDATE clause was - used). This is primarily useful for obtaining values that were + to compute and return value(s) based on each row actually inserted. + If an ON CONFLICT DO UPDATE clause was used, + RETURNING also returns tuples which were updated, and + in the presence of an ON CONFLICT DO SELECT clause all + input rows are returned. With a traditional INSERT, + the RETURNING clause is primarily useful for obtaining + values that were supplied by defaults, such as a serial sequence number. However, any expression using the table's columns is allowed. The syntax of the RETURNING list is identical to that of the output - list of SELECT. Only rows that were successfully + list of SELECT. If an ON CONFLICT DO SELECT + clause is not present, only rows that were successfully inserted or updated will be returned. For example, if a row was locked but not updated because an ON CONFLICT DO UPDATE ... WHERE clause condition was not satisfied, the - row will not be returned. + row will not be returned. ON CONFLICT DO SELECT + works similarly, except no update takes place. You must have INSERT privilege on a table in order to insert into it. If ON CONFLICT DO UPDATE is present, UPDATE privilege on the table is also - required. + 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. @@ -114,10 +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. + 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. @@ -341,7 +356,10 @@ INSERT INTO table_name [ AS INSERT, all old values will be NULL. However, for an INSERT with an ON CONFLICT DO UPDATE clause, the old - values may be non-NULL. + values may be non-NULL. Similarly, for + ON CONFLICT DO SELECT, both old and new values + represent the existing row (since no modification takes place), + so old and new will be identical for conflicting rows. @@ -377,6 +395,9 @@ INSERT INTO table_name [ AS ON CONFLICT DO UPDATE updates the existing row that conflicts with the row proposed for insertion as its alternative action. + ON CONFLICT DO SELECT returns the existing row + that conflicts with the row proposed for insertion, optionally + with row-level locking. @@ -408,6 +429,13 @@ INSERT INTO table_name [ AS . + + ON CONFLICT DO SELECT similarly allows an atomic + INSERT or SELECT outcome. This + is also known as a idempotent insert or + get or create. + + conflict_target @@ -421,7 +449,8 @@ INSERT INTO table_name [ AS conflict_target; when omitted, conflicts with all usable constraints (and unique indexes) are handled. For ON CONFLICT DO - UPDATE, a conflict_target + UPDATE and ON CONFLICT DO SELECT, + a conflict_target must be provided. @@ -433,10 +462,11 @@ INSERT INTO table_name [ AS conflict_action specifies an alternative ON CONFLICT action. It can be - either DO NOTHING, or a DO + either DO NOTHING, a DO UPDATE clause specifying the exact details of the UPDATE action to be performed in case of a - conflict. The SET and + conflict, or a DO SELECT clause that returns + the existing conflicting row. The SET and WHERE clauses in ON CONFLICT DO UPDATE have access to the existing row using the table's name (or an alias), and to the row proposed for insertion @@ -445,6 +475,18 @@ INSERT INTO table_name [ AS excluded columns are read. + + For ON CONFLICT DO SELECT, the optional + WHERE clause has access to the existing row + using the table's name (or an alias), and to the row proposed for + insertion using the special excluded table. + Only rows for which the WHERE clause returns + true will be returned. An optional + FOR UPDATE, FOR NO KEY UPDATE, + FOR SHARE, or FOR KEY SHARE + clause can be specified to lock the existing row using the + specified lock strength. + Note that the effects of all per-row BEFORE INSERT triggers are reflected in @@ -547,19 +589,21 @@ INSERT INTO table_name [ AS An expression that returns a value of type - boolean. Only rows for which this expression - returns true will be updated, although all - rows will be locked when the ON CONFLICT DO UPDATE - action is taken. Note that - condition is evaluated last, after - a conflict has been identified as a candidate to update. + boolean. For ON CONFLICT DO UPDATE, + only rows for which this expression returns true + will be updated, although all rows will be locked when the + ON CONFLICT DO UPDATE action is taken. + For ON CONFLICT DO SELECT, only rows for which + this expression returns true will be returned. + Note that condition is evaluated last, after + a conflict has been identified as a candidate to update or select. 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. @@ -616,7 +660,7 @@ INSERT INTO table_name [ AS oid count The count is the number of - rows inserted or updated. oid is always 0 (it + rows inserted, updated, or selected for return. oid is always 0 (it used to be the OID assigned to the inserted row if count was exactly one and the target table was declared WITH OIDS and 0 otherwise, but creating a table @@ -627,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. @@ -802,6 +845,36 @@ INSERT INTO distributors AS d (did, dname) VALUES (8, 'Anvil Distribution') -- index to arbitrate taking the DO NOTHING action) INSERT INTO distributors (did, dname) VALUES (9, 'Antwerp Design') ON CONFLICT ON CONSTRAINT distributors_pkey DO NOTHING; + + + + Insert new distributor if possible, otherwise return the existing + distributor row. Example assumes a unique index has been defined + that constrains values appearing in the did column. + This is useful for get-or-create patterns: + +INSERT INTO distributors (did, dname) VALUES (11, 'Global Electronics') + ON CONFLICT (did) DO SELECT + RETURNING *; + + + + Insert a new distributor if the name doesn't match, otherwise return + the existing row. This example uses the excluded + table in the WHERE clause to filter results: + +INSERT INTO distributors (did, dname) VALUES (12, 'Micro Devices Inc') + ON CONFLICT (did) DO SELECT WHERE dname = EXCLUDED.dname + RETURNING *; + + + + Insert a new distributor or return and lock the existing row for update. + This is useful when you need to ensure exclusive access to the row: + +INSERT INTO distributors (did, dname) VALUES (13, 'Advanced Systems') + ON CONFLICT (did) DO SELECT FOR UPDATE + RETURNING *; 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/commands/explain.c b/src/backend/commands/explain.c index 7e699f8595e..10e636d465e 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -4670,10 +4670,36 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors, if (node->onConflictAction != ONCONFLICT_NONE) { - ExplainPropertyText("Conflict Resolution", - node->onConflictAction == ONCONFLICT_NOTHING ? - "NOTHING" : "UPDATE", - es); + const char *resolution = NULL; + + if (node->onConflictAction == ONCONFLICT_NOTHING) + resolution = "NOTHING"; + else if (node->onConflictAction == ONCONFLICT_UPDATE) + resolution = "UPDATE"; + else + { + Assert(node->onConflictAction == ONCONFLICT_SELECT); + switch (node->onConflictLockStrength) + { + case LCS_NONE: + resolution = "SELECT"; + break; + case LCS_FORKEYSHARE: + resolution = "SELECT FOR KEY SHARE"; + break; + case LCS_FORSHARE: + resolution = "SELECT FOR SHARE"; + break; + case LCS_FORNOKEYUPDATE: + resolution = "SELECT FOR NO KEY UPDATE"; + break; + case LCS_FORUPDATE: + resolution = "SELECT FOR UPDATE"; + break; + } + } + + ExplainPropertyText("Conflict Resolution", resolution, es); /* * Don't display arbiter indexes at all when DO NOTHING variant @@ -4682,7 +4708,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors, if (idxNames) ExplainPropertyList("Conflict Arbiter Indexes", idxNames, es); - /* ON CONFLICT DO UPDATE WHERE qual is specially displayed */ + /* ON CONFLICT DO UPDATE/SELECT WHERE qual is specially displayed */ if (node->onConflictWhere) { show_upper_qual((List *) node->onConflictWhere, "Conflict Filter", diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 0dcce181f09..ab3c74c6fc5 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -732,20 +732,27 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, leaf_part_rri->ri_onConflictArbiterIndexes = arbiterIndexes; /* - * In the DO UPDATE case, we have some more state to initialize. + * In the DO UPDATE and DO SELECT cases, we have some more state to + * initialize. */ - if (node->onConflictAction == ONCONFLICT_UPDATE) + if (node->onConflictAction == ONCONFLICT_UPDATE || + node->onConflictAction == ONCONFLICT_SELECT) { - OnConflictSetState *onconfl = makeNode(OnConflictSetState); + OnConflictActionState *onconfl = makeNode(OnConflictActionState); TupleConversionMap *map; map = ExecGetRootToChildMap(leaf_part_rri, estate); - Assert(node->onConflictSet != NIL); + Assert(node->onConflictSet != NIL || + node->onConflictAction == ONCONFLICT_SELECT); Assert(rootResultRelInfo->ri_onConflict != NULL); leaf_part_rri->ri_onConflict = onconfl; + /* Lock strength for DO SELECT [FOR UPDATE/SHARE] */ + onconfl->oc_LockStrength = + rootResultRelInfo->ri_onConflict->oc_LockStrength; + /* * Need a separate existing slot for each partition, as the * partition could be of a different AM, even if the tuple @@ -758,7 +765,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, /* * If the partition's tuple descriptor matches exactly the root * parent (the common case), we can re-use most of the parent's ON - * CONFLICT SET state, skipping a bunch of work. Otherwise, we + * CONFLICT action state, skipping a bunch of work. Otherwise, we * need to create state specific to this partition. */ if (map == NULL) @@ -766,7 +773,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, /* * It's safe to reuse these from the partition root, as we * only process one tuple at a time (therefore we won't - * overwrite needed data in slots), and the results of + * overwrite needed data in slots), and the results of any * projections are independent of the underlying storage. * Projections and where clauses themselves don't store state * / are independent of the underlying storage. @@ -780,66 +787,81 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, } else { - List *onconflset; - List *onconflcols; - /* - * Translate expressions in onConflictSet to account for - * different attribute numbers. For that, map partition - * varattnos twice: first to catch the EXCLUDED - * pseudo-relation (INNER_VAR), and second to handle the main - * target relation (firstVarno). + * For ON CONFLICT DO UPDATE, translate expressions in + * onConflictSet to account for different attribute numbers. + * For that, map partition varattnos twice: first to catch the + * EXCLUDED pseudo-relation (INNER_VAR), and second to handle + * the main target relation (firstVarno). */ - onconflset = copyObject(node->onConflictSet); - if (part_attmap == NULL) - part_attmap = - build_attrmap_by_name(RelationGetDescr(partrel), - RelationGetDescr(firstResultRel), - false); - onconflset = (List *) - map_variable_attnos((Node *) onconflset, - INNER_VAR, 0, - part_attmap, - RelationGetForm(partrel)->reltype, - &found_whole_row); - /* We ignore the value of found_whole_row. */ - onconflset = (List *) - map_variable_attnos((Node *) onconflset, - firstVarno, 0, - part_attmap, - RelationGetForm(partrel)->reltype, - &found_whole_row); - /* We ignore the value of found_whole_row. */ - - /* Finally, adjust the target colnos to match the partition. */ - onconflcols = adjust_partition_colnos(node->onConflictCols, - leaf_part_rri); - - /* create the tuple slot for the UPDATE SET projection */ - onconfl->oc_ProjSlot = - table_slot_create(partrel, - &mtstate->ps.state->es_tupleTable); + if (node->onConflictAction == ONCONFLICT_UPDATE) + { + List *onconflset; + List *onconflcols; + + onconflset = copyObject(node->onConflictSet); + if (part_attmap == NULL) + part_attmap = + build_attrmap_by_name(RelationGetDescr(partrel), + RelationGetDescr(firstResultRel), + false); + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + INNER_VAR, 0, + part_attmap, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + firstVarno, 0, + part_attmap, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ - /* build UPDATE SET projection state */ - onconfl->oc_ProjInfo = - ExecBuildUpdateProjection(onconflset, - true, - onconflcols, - partrelDesc, - econtext, - onconfl->oc_ProjSlot, - &mtstate->ps); + /* + * Finally, adjust the target colnos to match the + * partition. + */ + onconflcols = adjust_partition_colnos(node->onConflictCols, + leaf_part_rri); + + /* create the tuple slot for the UPDATE SET projection */ + onconfl->oc_ProjSlot = + table_slot_create(partrel, + &mtstate->ps.state->es_tupleTable); + + /* build UPDATE SET projection state */ + onconfl->oc_ProjInfo = + ExecBuildUpdateProjection(onconflset, + true, + onconflcols, + partrelDesc, + econtext, + onconfl->oc_ProjSlot, + &mtstate->ps); + } /* - * 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); + clause = copyObject((List *) node->onConflictWhere); clause = (List *) map_variable_attnos((Node *) clause, diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index e44f1223886..9d3cd430084 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -147,12 +147,24 @@ static void ExecCrossPartitionUpdateForeignKey(ModifyTableContext *context, ItemPointer tupleid, TupleTableSlot *oldslot, TupleTableSlot *newslot); +static bool ExecOnConflictLockRow(ModifyTableContext *context, + TupleTableSlot *existing, + ItemPointer conflictTid, + Relation relation, + LockTupleMode lockmode, + bool isUpdate); static bool ExecOnConflictUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo, ItemPointer conflictTid, TupleTableSlot *excludedSlot, bool canSetTag, TupleTableSlot **returning); +static bool ExecOnConflictSelect(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + ItemPointer conflictTid, + TupleTableSlot *excludedSlot, + bool canSetTag, + TupleTableSlot **returning); static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate, EState *estate, PartitionTupleRouting *proute, @@ -1160,6 +1172,26 @@ ExecInsert(ModifyTableContext *context, else goto vlock; } + else if (onconflict == ONCONFLICT_SELECT) + { + /* + * In case of ON CONFLICT DO SELECT, optionally lock the + * conflicting tuple, fetch it and project RETURNING on + * it. Be prepared to retry if locking fails because of a + * concurrent UPDATE/DELETE to the conflict tuple. + */ + TupleTableSlot *returning = NULL; + + if (ExecOnConflictSelect(context, resultRelInfo, + &conflictTid, slot, canSetTag, + &returning)) + { + InstrCountTuples2(&mtstate->ps, 1); + return returning; + } + else + goto vlock; + } else { /* @@ -2701,52 +2733,32 @@ redo_act: } /* - * ExecOnConflictUpdate --- execute UPDATE of INSERT ON CONFLICT DO UPDATE + * ExecOnConflictLockRow --- lock the row for ON CONFLICT DO UPDATE/SELECT * - * Try to lock tuple for update as part of speculative insertion. If - * a qual originating from ON CONFLICT DO UPDATE is satisfied, update - * (but still lock row, even though it may not satisfy estate's - * snapshot). + * Try to lock tuple for update as part of speculative insertion for ON + * CONFLICT DO UPDATE or ON CONFLICT DO SELECT FOR UPDATE/SHARE. * - * Returns true if we're done (with or without an update), or false if - * the caller must retry the INSERT from scratch. + * Returns true if the row is successfully locked, or false if the caller must + * retry the INSERT from scratch. */ static bool -ExecOnConflictUpdate(ModifyTableContext *context, - ResultRelInfo *resultRelInfo, - ItemPointer conflictTid, - TupleTableSlot *excludedSlot, - bool canSetTag, - TupleTableSlot **returning) +ExecOnConflictLockRow(ModifyTableContext *context, + TupleTableSlot *existing, + ItemPointer conflictTid, + Relation relation, + LockTupleMode lockmode, + bool isUpdate) { - ModifyTableState *mtstate = context->mtstate; - ExprContext *econtext = mtstate->ps.ps_ExprContext; - Relation relation = resultRelInfo->ri_RelationDesc; - ExprState *onConflictSetWhere = resultRelInfo->ri_onConflict->oc_WhereClause; - TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing; TM_FailureData tmfd; - LockTupleMode lockmode; TM_Result test; Datum xminDatum; TransactionId xmin; bool isnull; /* - * Parse analysis should have blocked ON CONFLICT for all system - * relations, which includes these. There's no fundamental obstacle to - * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other - * ExecUpdate() caller. - */ - Assert(!resultRelInfo->ri_needLockTagTuple); - - /* Determine lock mode to use */ - lockmode = ExecUpdateLockMode(context->estate, resultRelInfo); - - /* - * Lock tuple for update. Don't follow updates when tuple cannot be - * locked without doing so. A row locking conflict here means our - * previous conclusion that the tuple is conclusively committed is not - * true anymore. + * Don't follow updates when tuple cannot be locked without doing so. A + * row locking conflict here means our previous conclusion that the tuple + * is conclusively committed is not true anymore. */ test = table_tuple_lock(relation, conflictTid, context->estate->es_snapshot, @@ -2788,7 +2800,7 @@ ExecOnConflictUpdate(ModifyTableContext *context, (errcode(ERRCODE_CARDINALITY_VIOLATION), /* translator: %s is a SQL command name */ errmsg("%s command cannot affect row a second time", - "ON CONFLICT DO UPDATE"), + isUpdate ? "ON CONFLICT DO UPDATE" : "ON CONFLICT DO SELECT"), errhint("Ensure that no rows proposed for insertion within the same command have duplicate constrained values."))); /* This shouldn't happen */ @@ -2845,6 +2857,50 @@ ExecOnConflictUpdate(ModifyTableContext *context, } /* Success, the tuple is locked. */ + return true; +} + +/* + * ExecOnConflictUpdate --- execute UPDATE of INSERT ON CONFLICT DO UPDATE + * + * Try to lock tuple for update as part of speculative insertion. If + * a qual originating from ON CONFLICT DO UPDATE is satisfied, update + * (but still lock row, even though it may not satisfy estate's + * snapshot). + * + * Returns true if we're done (with or without an update), or false if + * the caller must retry the INSERT from scratch. + */ +static bool +ExecOnConflictUpdate(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + ItemPointer conflictTid, + TupleTableSlot *excludedSlot, + bool canSetTag, + TupleTableSlot **returning) +{ + ModifyTableState *mtstate = context->mtstate; + ExprContext *econtext = mtstate->ps.ps_ExprContext; + Relation relation = resultRelInfo->ri_RelationDesc; + ExprState *onConflictSetWhere = resultRelInfo->ri_onConflict->oc_WhereClause; + TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing; + LockTupleMode lockmode; + + /* + * Parse analysis should have blocked ON CONFLICT for all system + * relations, which includes these. There's no fundamental obstacle to + * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other + * ExecUpdate() caller. + */ + Assert(!resultRelInfo->ri_needLockTagTuple); + + /* Determine lock mode to use */ + lockmode = ExecUpdateLockMode(context->estate, resultRelInfo); + + /* Lock tuple for update */ + if (!ExecOnConflictLockRow(context, existing, conflictTid, + resultRelInfo->ri_RelationDesc, lockmode, true)) + return false; /* * Verify that the tuple is visible to our MVCC snapshot if the current @@ -2886,11 +2942,12 @@ ExecOnConflictUpdate(ModifyTableContext *context, * security barrier quals (if any), enforced here as RLS checks/WCOs. * * The rewriter creates UPDATE RLS checks/WCOs for UPDATE security - * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK, - * but that's almost the extent of its special handling for ON - * CONFLICT DO UPDATE. + * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK. If + * SELECT rights are required on the target table, the rewriter also + * adds SELECT RLS checks/WCOs for SELECT security quals, using WCOs + * of the same kind, so this check enforces them too. * - * The rewriter will also have associated UPDATE applicable straight + * The rewriter will also have associated UPDATE-applicable straight * RLS checks/WCOs for the benefit of the ExecUpdate() call that * follows. INSERTs and UPDATEs naturally have mutually exclusive WCO * kinds, so there is no danger of spurious over-enforcement in the @@ -2935,6 +2992,140 @@ ExecOnConflictUpdate(ModifyTableContext *context, return true; } +/* + * ExecOnConflictSelect --- execute SELECT of INSERT ON CONFLICT DO SELECT + * + * If SELECT FOR UPDATE/SHARE is specified, try to lock tuple as part of + * speculative insertion. If a qual originating from ON CONFLICT DO SELECT is + * satisfied, select the row. + * + * Returns true if we're done (with or without a select), or false if the + * caller must retry the INSERT from scratch. + */ +static bool +ExecOnConflictSelect(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + ItemPointer conflictTid, + TupleTableSlot *excludedSlot, + bool canSetTag, + TupleTableSlot **rslot) +{ + ModifyTableState *mtstate = context->mtstate; + ExprContext *econtext = mtstate->ps.ps_ExprContext; + Relation relation = resultRelInfo->ri_RelationDesc; + ExprState *onConflictSelectWhere = resultRelInfo->ri_onConflict->oc_WhereClause; + TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing; + LockClauseStrength lockStrength = resultRelInfo->ri_onConflict->oc_LockStrength; + + /* + * Parse analysis should have blocked ON CONFLICT for all system + * relations, which includes these. There's no fundamental obstacle to + * supporting this; we'd just need to handle LOCKTAG_TUPLE like the other + * ExecUpdate() caller. + */ + Assert(!resultRelInfo->ri_needLockTagTuple); + + 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; + + switch (lockStrength) + { + case LCS_FORKEYSHARE: + lockmode = LockTupleKeyShare; + break; + case LCS_FORSHARE: + lockmode = LockTupleShare; + break; + case LCS_FORNOKEYUPDATE: + lockmode = LockTupleNoKeyExclusive; + break; + case LCS_FORUPDATE: + lockmode = LockTupleExclusive; + break; + default: + elog(ERROR, "unexpected lock strength %d", lockStrength); + } + + if (!ExecOnConflictLockRow(context, existing, conflictTid, + resultRelInfo->ri_RelationDesc, lockmode, false)) + return false; + } + + /* + * For the same reasons as ExecOnConflictUpdate, we must verify that the + * tuple is visible to our snapshot. + */ + ExecCheckTupleVisible(context->estate, relation, existing); + + /* + * Make tuple and any needed join variables available to ExecQual. The + * EXCLUDED tuple is installed in ecxt_innertuple, while the target's + * existing tuple is installed in the scantuple. EXCLUDED has been made + * to reference INNER_VAR in setrefs.c, but there is no other redirection. + */ + econtext->ecxt_scantuple = existing; + econtext->ecxt_innertuple = excludedSlot; + econtext->ecxt_outertuple = NULL; + + if (!ExecQual(onConflictSelectWhere, econtext)) + { + ExecClearTuple(existing); /* see return below */ + InstrCountFiltered1(&mtstate->ps, 1); + return true; /* done with the tuple */ + } + + if (resultRelInfo->ri_WithCheckOptions != NIL) + { + /* + * Check target's existing tuple against SELECT-applicable USING + * security barrier quals (if any), enforced here as RLS checks/WCOs. + * + * The rewriter creates SELECT RLS checks/WCOs for SELECT security + * quals, and stores them as WCOs of "kind" WCO_RLS_CONFLICT_CHECK. If + * FOR UPDATE/SHARE was specified, UPDATE rights are required on the + * target table, and the rewriter also adds UPDATE RLS checks/WCOs for + * UPDATE security quals, using WCOs of the same kind, so this check + * enforces them too. + */ + ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo, + existing, + mtstate->ps.state); + } + + /* Parse analysis should already have disallowed this, as RETURNING + * is required for DO SELECT. + */ + Assert(resultRelInfo->ri_projectReturning); + + *rslot = ExecProcessReturning(context, resultRelInfo, CMD_INSERT, + existing, existing, context->planSlot); + + if (canSetTag) + context->estate->es_processed++; + + /* + * Before releasing the existing tuple, make sure rslot has a local copy + * of any pass-by-reference values. + */ + ExecMaterializeSlot(*rslot); + + /* + * Clear out existing tuple, as there might not be another conflict among + * the next input rows. Don't want to hold resources till the end of the + * query. + */ + ExecClearTuple(existing); + + return true; +} + /* * Perform MERGE. */ @@ -5030,49 +5221,60 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } /* - * If needed, Initialize target list, projection and qual for ON CONFLICT - * DO UPDATE. + * For ON CONFLICT DO UPDATE/SELECT, initialize the ON CONFLICT action + * state. */ - if (node->onConflictAction == ONCONFLICT_UPDATE) + if (node->onConflictAction == ONCONFLICT_UPDATE || + node->onConflictAction == ONCONFLICT_SELECT) { - OnConflictSetState *onconfl = makeNode(OnConflictSetState); - ExprContext *econtext; - TupleDesc relationDesc; + OnConflictActionState *onconfl = makeNode(OnConflictActionState); /* already exists if created by RETURNING processing above */ if (mtstate->ps.ps_ExprContext == NULL) ExecAssignExprContext(estate, &mtstate->ps); - econtext = mtstate->ps.ps_ExprContext; - relationDesc = resultRelInfo->ri_RelationDesc->rd_att; - - /* create state for DO UPDATE SET operation */ + /* action state for DO UPDATE/SELECT */ resultRelInfo->ri_onConflict = onconfl; + /* lock strength for DO SELECT [FOR UPDATE/SHARE] */ + onconfl->oc_LockStrength = node->onConflictLockStrength; + /* initialize slot for the existing tuple */ onconfl->oc_Existing = table_slot_create(resultRelInfo->ri_RelationDesc, &mtstate->ps.state->es_tupleTable); /* - * Create the tuple slot for the UPDATE SET projection. We want a slot - * of the table's type here, because the slot will be used to insert - * into the table, and for RETURNING processing - which may access - * system attributes. + * For ON CONFLICT DO UPDATE, initialize target list and projection. */ - onconfl->oc_ProjSlot = - table_slot_create(resultRelInfo->ri_RelationDesc, - &mtstate->ps.state->es_tupleTable); + if (node->onConflictAction == ONCONFLICT_UPDATE) + { + ExprContext *econtext; + TupleDesc relationDesc; + + econtext = mtstate->ps.ps_ExprContext; + relationDesc = resultRelInfo->ri_RelationDesc->rd_att; - /* build UPDATE SET projection state */ - onconfl->oc_ProjInfo = - ExecBuildUpdateProjection(node->onConflictSet, - true, - node->onConflictCols, - relationDesc, - econtext, - onconfl->oc_ProjSlot, - &mtstate->ps); + /* + * Create the tuple slot for the UPDATE SET projection. We want a + * slot of the table's type here, because the slot will be used to + * insert into the table, and for RETURNING processing - which may + * access system attributes. + */ + onconfl->oc_ProjSlot = + table_slot_create(resultRelInfo->ri_RelationDesc, + &mtstate->ps.state->es_tupleTable); + + /* build UPDATE SET projection state */ + onconfl->oc_ProjInfo = + ExecBuildUpdateProjection(node->onConflictSet, + true, + node->onConflictCols, + relationDesc, + econtext, + onconfl->oc_ProjSlot, + &mtstate->ps); + } /* initialize state to evaluate the WHERE clause, if any */ if (node->onConflictWhere) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 8af091ba647..8f2586eeda1 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -7036,10 +7036,11 @@ make_modifytable(PlannerInfo *root, Plan *subplan, if (!onconflict) { node->onConflictAction = ONCONFLICT_NONE; + node->arbiterIndexes = NIL; + node->onConflictLockStrength = LCS_NONE; node->onConflictSet = NIL; node->onConflictCols = NIL; node->onConflictWhere = NULL; - node->arbiterIndexes = NIL; node->exclRelRTI = 0; node->exclRelTlist = NIL; } @@ -7047,6 +7048,9 @@ make_modifytable(PlannerInfo *root, Plan *subplan, { node->onConflictAction = onconflict->action; + /* Lock strength for ON CONFLICT SELECT [FOR UPDATE/SHARE] */ + node->onConflictLockStrength = onconflict->lockStrength; + /* * Here we convert the ON CONFLICT UPDATE tlist, if any, to the * executor's convention of having consecutive resno's. The actual diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index ccdc9bc264a..b4d9a998e07 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -1140,7 +1140,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) * those are already used by RETURNING and it seems better to * be non-conflicting. */ - if (splan->onConflictSet) + if (splan->onConflictAction == ONCONFLICT_UPDATE || + splan->onConflictAction == ONCONFLICT_SELECT) { indexed_tlist *itlist; diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 7af9a2064e3..c1a00c354ec 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -939,10 +939,18 @@ infer_arbiter_indexes(PlannerInfo *root) */ if (indexOidFromConstraint == idxForm->indexrelid) { - if (idxForm->indisexclusion && onconflict->action == ONCONFLICT_UPDATE) + /* + * ON CONFLICT DO UPDATE/SELECT are not supported with exclusion + * constraints (they require a unique index, to ensure that there + * is only one conflicting row to update/select). + */ + if (idxForm->indisexclusion && + (onconflict->action == ONCONFLICT_UPDATE || + onconflict->action == ONCONFLICT_SELECT)) ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints"))); + errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("ON CONFLICT DO %s not supported with exclusion constraints", + onconflict->action == ONCONFLICT_UPDATE ? "UPDATE" : "SELECT")); results = lappend_oid(results, idxForm->indexrelid); foundValid |= idxForm->indisvalid; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 7843a0c857e..73b3a0fc938 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -650,7 +650,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) ListCell *icols; ListCell *attnos; ListCell *lc; - bool isOnConflictUpdate; + bool requiresUpdatePerm; AclMode targetPerms; /* There can't be any outer WITH to worry about */ @@ -669,8 +669,14 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) qry->override = stmt->override; - isOnConflictUpdate = (stmt->onConflictClause && - stmt->onConflictClause->action == ONCONFLICT_UPDATE); + /* + * ON CONFLICT DO UPDATE and ON CONFLICT DO SELECT FOR UPDATE/SHARE + * require UPDATE permission on the target relation. + */ + requiresUpdatePerm = (stmt->onConflictClause && + (stmt->onConflictClause->action == ONCONFLICT_UPDATE || + (stmt->onConflictClause->action == ONCONFLICT_SELECT && + stmt->onConflictClause->lockStrength != LCS_NONE))); /* * We have three cases to deal with: DEFAULT VALUES (selectStmt == NULL), @@ -720,7 +726,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) * to the joinlist or namespace. */ targetPerms = ACL_INSERT; - if (isOnConflictUpdate) + if (requiresUpdatePerm) targetPerms |= ACL_UPDATE; qry->resultRelation = setTargetTable(pstate, stmt->relation, false, false, targetPerms); @@ -1027,6 +1033,15 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) false, true, true); } + /* ON CONFLICT DO SELECT requires a RETURNING clause */ + if (stmt->onConflictClause && + stmt->onConflictClause->action == ONCONFLICT_SELECT && + !stmt->returningClause) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"), + parser_errposition(pstate, stmt->onConflictClause->location)); + /* Process ON CONFLICT, if any. */ if (stmt->onConflictClause) qry->onConflict = transformOnConflictClause(pstate, @@ -1185,12 +1200,13 @@ transformOnConflictClause(ParseState *pstate, OnConflictExpr *result; /* - * If this is ON CONFLICT ... UPDATE, first create the range table entry - * for the EXCLUDED pseudo relation, so that that will be present while - * processing arbiter expressions. (You can't actually reference it from - * there, but this provides a useful error message if you try.) + * If this is ON CONFLICT ... UPDATE/SELECT, first create the range table + * entry for the EXCLUDED pseudo relation, so that that will be present + * while processing arbiter expressions. (You can't actually reference it + * from there, but this provides a useful error message if you try.) */ - if (onConflictClause->action == ONCONFLICT_UPDATE) + if (onConflictClause->action == ONCONFLICT_UPDATE || + onConflictClause->action == ONCONFLICT_SELECT) { Relation targetrel = pstate->p_target_relation; RangeTblEntry *exclRte; @@ -1219,27 +1235,28 @@ transformOnConflictClause(ParseState *pstate, transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems, &arbiterWhere, &arbiterConstraint); - /* Process DO UPDATE */ - if (onConflictClause->action == ONCONFLICT_UPDATE) + /* Process DO UPDATE/SELECT */ + if (onConflictClause->action == ONCONFLICT_UPDATE || + onConflictClause->action == ONCONFLICT_SELECT) { - /* - * Expressions in the UPDATE targetlist need to be handled like UPDATE - * not INSERT. We don't need to save/restore this because all INSERT - * expressions have been parsed already. - */ - pstate->p_is_insert = false; - /* * Add the EXCLUDED pseudo relation to the query namespace, making it - * available in the UPDATE subexpressions. + * available in the UPDATE/SELECT subexpressions. */ addNSItemToQuery(pstate, exclNSItem, false, true, true); - /* - * Now transform the UPDATE subexpressions. - */ - onConflictSet = - transformUpdateTargetList(pstate, onConflictClause->targetList); + if (onConflictClause->action == ONCONFLICT_UPDATE) + { + /* + * Expressions in the UPDATE targetlist need to be handled like + * UPDATE not INSERT. We don't need to save/restore this because + * all INSERT expressions have been parsed already. + */ + pstate->p_is_insert = false; + + onConflictSet = + transformUpdateTargetList(pstate, onConflictClause->targetList); + } onConflictWhere = transformWhereClause(pstate, onConflictClause->whereClause, @@ -1254,13 +1271,14 @@ transformOnConflictClause(ParseState *pstate, pstate->p_namespace = list_delete_last(pstate->p_namespace); } - /* Finally, build ON CONFLICT DO [NOTHING | UPDATE] expression */ + /* Finally, build ON CONFLICT DO [NOTHING | SELECT | UPDATE] expression */ result = makeNode(OnConflictExpr); result->action = onConflictClause->action; result->arbiterElems = arbiterElems; result->arbiterWhere = arbiterWhere; result->constraint = arbiterConstraint; + result->lockStrength = onConflictClause->lockStrength; result->onConflictSet = onConflictSet; result->onConflictWhere = onConflictWhere; result->exclRelIndex = exclRelIndex; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index c3a0a354a9c..13e6c0de342 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -480,7 +480,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type OptNoLog %type OnCommitOption -%type for_locking_strength +%type for_locking_strength opt_for_locking_strength %type for_locking_item %type for_locking_clause opt_for_locking_clause for_locking_items %type locked_rels_list @@ -12439,12 +12439,24 @@ insert_column_item: ; opt_on_conflict: + ON CONFLICT opt_conf_expr DO SELECT opt_for_locking_strength where_clause + { + $$ = makeNode(OnConflictClause); + $$->action = ONCONFLICT_SELECT; + $$->infer = $3; + $$->targetList = NIL; + $$->lockStrength = $6; + $$->whereClause = $7; + $$->location = @1; + } + | ON CONFLICT opt_conf_expr DO UPDATE SET set_clause_list where_clause { $$ = makeNode(OnConflictClause); $$->action = ONCONFLICT_UPDATE; $$->infer = $3; $$->targetList = $7; + $$->lockStrength = LCS_NONE; $$->whereClause = $8; $$->location = @1; } @@ -12455,6 +12467,7 @@ opt_on_conflict: $$->action = ONCONFLICT_NOTHING; $$->infer = $3; $$->targetList = NIL; + $$->lockStrength = LCS_NONE; $$->whereClause = NULL; $$->location = @1; } @@ -13684,6 +13697,11 @@ for_locking_strength: | FOR KEY SHARE { $$ = LCS_FORKEYSHARE; } ; +opt_for_locking_strength: + for_locking_strength { $$ = $1; } + | /* EMPTY */ { $$ = LCS_NONE; } + ; + locked_rels_list: OF qualified_name_list { $$ = $2; } | /* EMPTY */ { $$ = NIL; } diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index ca26f6f61f2..e8d1e01732e 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -3368,13 +3368,15 @@ transformOnConflictArbiter(ParseState *pstate, *arbiterWhere = NULL; *constraint = InvalidOid; - if (onConflictClause->action == ONCONFLICT_UPDATE && !infer) + if ((onConflictClause->action == ONCONFLICT_UPDATE || + onConflictClause->action == ONCONFLICT_SELECT) && !infer) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("ON CONFLICT DO UPDATE requires inference specification or constraint name"), - errhint("For example, ON CONFLICT (column_name)."), - parser_errposition(pstate, - exprLocation((Node *) onConflictClause)))); + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("ON CONFLICT DO %s requires inference specification or constraint name", + onConflictClause->action == ONCONFLICT_UPDATE ? "UPDATE" : "SELECT"), + errhint("For example, ON CONFLICT (column_name)."), + parser_errposition(pstate, + exprLocation((Node *) onConflictClause))); /* * To simplify certain aspects of its design, speculative insertion into diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index adc9e7600e1..aa377022df1 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -655,6 +655,19 @@ rewriteRuleAction(Query *parsetree, rule_action = sub_action; } + /* + * If rule_action is INSERT .. ON CONFLICT DO SELECT, the parser should + * have verified that it has a RETURNING clause, but we must also check + * that the triggering query has a RETURNING clause. + */ + if (rule_action->onConflict && + rule_action->onConflict->action == ONCONFLICT_SELECT && + (!rule_action->returningList || !parsetree->returningList)) + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("ON CONFLICT DO SELECT requires a RETURNING clause"), + errdetail("A rule action is INSERT ... ON CONFLICT DO SELECT, which requires a RETURNING clause.")); + /* * If rule_action has a RETURNING clause, then either throw it away if the * triggering query has no RETURNING clause, or rewrite it to emit what @@ -3640,11 +3653,12 @@ rewriteTargetView(Query *parsetree, Relation view) } /* - * For INSERT .. ON CONFLICT .. DO UPDATE, we must also update assorted - * stuff in the onConflict data structure. + * For INSERT .. ON CONFLICT .. DO UPDATE/SELECT, we must also update + * assorted stuff in the onConflict data structure. */ if (parsetree->onConflict && - parsetree->onConflict->action == ONCONFLICT_UPDATE) + (parsetree->onConflict->action == ONCONFLICT_UPDATE || + parsetree->onConflict->action == ONCONFLICT_SELECT)) { Index old_exclRelIndex, new_exclRelIndex; @@ -3653,9 +3667,8 @@ rewriteTargetView(Query *parsetree, Relation view) List *tmp_tlist; /* - * Like the INSERT/UPDATE code above, update the resnos in the - * auxiliary UPDATE targetlist to refer to columns of the base - * relation. + * For ON CONFLICT DO UPDATE, update the resnos in the auxiliary + * UPDATE targetlist to refer to columns of the base relation. */ foreach(lc, parsetree->onConflict->onConflictSet) { @@ -3674,7 +3687,7 @@ rewriteTargetView(Query *parsetree, Relation view) } /* - * Also, create a new RTE for the EXCLUDED pseudo-relation, using the + * Create a new RTE for the EXCLUDED pseudo-relation, using the * query's new base rel (which may well have a different column list * from the view, hence we need a new column alias list). This should * match transformOnConflictClause. In particular, note that the diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 4dad384d04d..c9bdff6f8f5 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -301,40 +301,48 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, } /* - * For INSERT ... ON CONFLICT DO UPDATE we need additional policy - * checks for the UPDATE which may be applied to the same RTE. + * For INSERT ... ON CONFLICT DO UPDATE/SELECT we need additional + * policy checks for the UPDATE/SELECT which may be applied to the + * same RTE. */ - if (commandType == CMD_INSERT && - root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE) + if (commandType == CMD_INSERT && root->onConflict && + (root->onConflict->action == ONCONFLICT_UPDATE || + root->onConflict->action == ONCONFLICT_SELECT)) { - List *conflict_permissive_policies; - List *conflict_restrictive_policies; + List *conflict_permissive_policies = NIL; + List *conflict_restrictive_policies = NIL; List *conflict_select_permissive_policies = NIL; List *conflict_select_restrictive_policies = NIL; - /* Get the policies that apply to the auxiliary UPDATE */ - get_policies_for_relation(rel, CMD_UPDATE, user_id, - &conflict_permissive_policies, - &conflict_restrictive_policies); - - /* - * Enforce the USING clauses of the UPDATE policies using WCOs - * rather than security quals. This ensures that an error is - * raised if the conflicting row cannot be updated due to RLS, - * rather than the change being silently dropped. - */ - add_with_check_options(rel, rt_index, - WCO_RLS_CONFLICT_CHECK, - conflict_permissive_policies, - conflict_restrictive_policies, - withCheckOptions, - hasSubLinks, - true); + if (perminfo->requiredPerms & ACL_UPDATE) + { + /* + * Get the policies that apply to the auxiliary UPDATE or + * SELECT FOR SHARE/UDPATE. + */ + get_policies_for_relation(rel, CMD_UPDATE, user_id, + &conflict_permissive_policies, + &conflict_restrictive_policies); + + /* + * Enforce the USING clauses of the UPDATE policies using WCOs + * rather than security quals. This ensures that an error is + * raised if the conflicting row cannot be updated/locked due + * to RLS, rather than the change being silently dropped. + */ + add_with_check_options(rel, rt_index, + WCO_RLS_CONFLICT_CHECK, + conflict_permissive_policies, + conflict_restrictive_policies, + withCheckOptions, + hasSubLinks, + true); + } /* * Get and add ALL/SELECT policies, as WCO_RLS_CONFLICT_CHECK WCOs - * to ensure they are considered when taking the UPDATE path of an - * INSERT .. ON CONFLICT DO UPDATE, if SELECT rights are required + * to ensure they are considered when taking the UPDATE/SELECT + * path of an INSERT .. ON CONFLICT, if SELECT rights are required * for this relation, also as WCO policies, again, to avoid * silently dropping data. See above. */ @@ -352,29 +360,36 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, true); } - /* Enforce the WITH CHECK clauses of the UPDATE policies */ - add_with_check_options(rel, rt_index, - WCO_RLS_UPDATE_CHECK, - conflict_permissive_policies, - conflict_restrictive_policies, - withCheckOptions, - hasSubLinks, - false); - /* - * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure - * that the final updated row is visible when taking the UPDATE - * path of an INSERT .. ON CONFLICT DO UPDATE, if SELECT rights - * are required for this relation. + * For INSERT .. ON CONFLICT DO UPDATE, add additional policies to + * be checked when the auxiliary UPDATE is executed. */ - if (perminfo->requiredPerms & ACL_SELECT) + if (root->onConflict->action == ONCONFLICT_UPDATE) + { + /* Enforce the WITH CHECK clauses of the UPDATE policies */ add_with_check_options(rel, rt_index, WCO_RLS_UPDATE_CHECK, - conflict_select_permissive_policies, - conflict_select_restrictive_policies, + conflict_permissive_policies, + conflict_restrictive_policies, withCheckOptions, hasSubLinks, - true); + false); + + /* + * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to + * ensure that the final updated row is visible when taking + * the UPDATE path of an INSERT .. ON CONFLICT, if SELECT + * rights are required for this relation. + */ + if (perminfo->requiredPerms & ACL_SELECT) + add_with_check_options(rel, rt_index, + WCO_RLS_UPDATE_CHECK, + conflict_select_permissive_policies, + conflict_select_restrictive_policies, + withCheckOptions, + hasSubLinks, + true); + } } } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 556ab057e5a..5da4b0ff296 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -426,6 +426,7 @@ static void get_update_query_targetlist_def(Query *query, List *targetList, static void get_delete_query_def(Query *query, deparse_context *context); static void get_merge_query_def(Query *query, deparse_context *context); static void get_utility_query_def(Query *query, deparse_context *context); +static char *get_lock_clause_strength(LockClauseStrength strength); static void get_basic_select_query(Query *query, deparse_context *context); static void get_target_list(List *targetList, deparse_context *context); static void get_returning_clause(Query *query, deparse_context *context); @@ -5997,30 +5998,9 @@ get_select_query_def(Query *query, deparse_context *context) if (rc->pushedDown) continue; - switch (rc->strength) - { - case LCS_NONE: - /* we intentionally throw an error for LCS_NONE */ - elog(ERROR, "unrecognized LockClauseStrength %d", - (int) rc->strength); - break; - case LCS_FORKEYSHARE: - appendContextKeyword(context, " FOR KEY SHARE", - -PRETTYINDENT_STD, PRETTYINDENT_STD, 0); - break; - case LCS_FORSHARE: - appendContextKeyword(context, " FOR SHARE", - -PRETTYINDENT_STD, PRETTYINDENT_STD, 0); - break; - case LCS_FORNOKEYUPDATE: - appendContextKeyword(context, " FOR NO KEY UPDATE", - -PRETTYINDENT_STD, PRETTYINDENT_STD, 0); - break; - case LCS_FORUPDATE: - appendContextKeyword(context, " FOR UPDATE", - -PRETTYINDENT_STD, PRETTYINDENT_STD, 0); - break; - } + appendContextKeyword(context, + get_lock_clause_strength(rc->strength), + -PRETTYINDENT_STD, PRETTYINDENT_STD, 0); appendStringInfo(buf, " OF %s", quote_identifier(get_rtable_name(rc->rti, @@ -6033,6 +6013,28 @@ get_select_query_def(Query *query, deparse_context *context) } } +static char * +get_lock_clause_strength(LockClauseStrength strength) +{ + switch (strength) + { + case LCS_NONE: + /* we intentionally throw an error for LCS_NONE */ + elog(ERROR, "unrecognized LockClauseStrength %d", + (int) strength); + break; + case LCS_FORKEYSHARE: + return " FOR KEY SHARE"; + case LCS_FORSHARE: + return " FOR SHARE"; + case LCS_FORNOKEYUPDATE: + return " FOR NO KEY UPDATE"; + case LCS_FORUPDATE: + return " FOR UPDATE"; + } + return NULL; /* keep compiler quiet */ +} + /* * Detect whether query looks like SELECT ... FROM VALUES(), * with no need to rename the output columns of the VALUES RTE. @@ -7125,7 +7127,7 @@ get_insert_query_def(Query *query, deparse_context *context) { appendStringInfoString(buf, " DO NOTHING"); } - else + else if (confl->action == ONCONFLICT_UPDATE) { appendStringInfoString(buf, " DO UPDATE SET "); /* Deparse targetlist */ @@ -7140,6 +7142,23 @@ get_insert_query_def(Query *query, deparse_context *context) get_rule_expr(confl->onConflictWhere, context, false); } } + else + { + Assert(confl->action == ONCONFLICT_SELECT); + appendStringInfoString(buf, " DO SELECT"); + + /* Add FOR [KEY] UPDATE/SHARE clause if present */ + if (confl->lockStrength != LCS_NONE) + appendStringInfoString(buf, get_lock_clause_strength(confl->lockStrength)); + + /* Add a WHERE clause if given */ + if (confl->onConflictWhere != NULL) + { + appendContextKeyword(context, " WHERE ", + -PRETTYINDENT_STD, PRETTYINDENT_STD, 1); + get_rule_expr(confl->onConflictWhere, context, false); + } + } } /* Add RETURNING if present */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 18ae8f0d4bb..cd5582f2485 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -422,19 +422,20 @@ typedef struct JunkFilter } JunkFilter; /* - * OnConflictSetState + * OnConflictActionState * - * Executor state of an ON CONFLICT DO UPDATE operation. + * Executor state of an ON CONFLICT DO UPDATE/SELECT operation. */ -typedef struct OnConflictSetState +typedef struct OnConflictActionState { NodeTag type; TupleTableSlot *oc_Existing; /* slot to store existing target tuple in */ TupleTableSlot *oc_ProjSlot; /* CONFLICT ... SET ... projection target */ ProjectionInfo *oc_ProjInfo; /* for ON CONFLICT DO UPDATE SET */ + LockClauseStrength oc_LockStrength; /* lock strength for DO SELECT */ ExprState *oc_WhereClause; /* state for the WHERE clause */ -} OnConflictSetState; +} OnConflictActionState; /* ---------------- * MergeActionState information @@ -579,8 +580,8 @@ typedef struct ResultRelInfo /* list of arbiter indexes to use to check conflicts */ List *ri_onConflictArbiterIndexes; - /* ON CONFLICT evaluation state */ - OnConflictSetState *ri_onConflict; + /* ON CONFLICT evaluation state for DO UPDATE/SELECT */ + OnConflictActionState *ri_onConflict; /* for MERGE, lists of MergeActionState (one per MergeMatchKind) */ List *ri_MergeActions[NUM_MERGE_MATCH_KINDS]; diff --git a/src/include/nodes/lockoptions.h b/src/include/nodes/lockoptions.h index 0b534e30603..59434fd480e 100644 --- a/src/include/nodes/lockoptions.h +++ b/src/include/nodes/lockoptions.h @@ -20,7 +20,8 @@ */ typedef enum LockClauseStrength { - LCS_NONE, /* no such clause - only used in PlanRowMark */ + LCS_NONE, /* no such clause - only used in PlanRowMark + * and ON CONFLICT SELECT */ LCS_FORKEYSHARE, /* FOR KEY SHARE */ LCS_FORSHARE, /* FOR SHARE */ LCS_FORNOKEYUPDATE, /* FOR NO KEY UPDATE */ diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index fb3957e75e5..691b5d385d6 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -428,6 +428,7 @@ typedef enum OnConflictAction ONCONFLICT_NONE, /* No "ON CONFLICT" clause */ ONCONFLICT_NOTHING, /* ON CONFLICT ... DO NOTHING */ ONCONFLICT_UPDATE, /* ON CONFLICT ... DO UPDATE */ + ONCONFLICT_SELECT, /* ON CONFLICT ... DO SELECT */ } OnConflictAction; /* diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d14294a4ece..4db404f5429 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -200,7 +200,7 @@ typedef struct Query /* OVERRIDING clause */ OverridingKind override pg_node_attr(query_jumble_ignore); - OnConflictExpr *onConflict; /* ON CONFLICT DO [NOTHING | UPDATE] */ + OnConflictExpr *onConflict; /* ON CONFLICT DO NOTHING/UPDATE/SELECT */ /* * The following three fields describe the contents of the RETURNING list @@ -1652,9 +1652,10 @@ typedef struct InferClause typedef struct OnConflictClause { NodeTag type; - OnConflictAction action; /* DO NOTHING or UPDATE? */ + OnConflictAction action; /* DO NOTHING, SELECT, or UPDATE */ InferClause *infer; /* Optional index inference clause */ - List *targetList; /* the target list (of ResTarget) */ + LockClauseStrength lockStrength; /* lock strength for DO SELECT */ + List *targetList; /* target list (of ResTarget) for DO UPDATE */ Node *whereClause; /* qualifications */ ParseLoc location; /* token location, or -1 if unknown */ } OnConflictClause; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index c4393a94321..7b9debccb0f 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -362,11 +362,13 @@ typedef struct ModifyTable OnConflictAction onConflictAction; /* List of ON CONFLICT arbiter index OIDs */ List *arbiterIndexes; + /* lock strength for ON CONFLICT SELECT */ + LockClauseStrength onConflictLockStrength; /* INSERT ON CONFLICT DO UPDATE targetlist */ List *onConflictSet; /* target column numbers for onConflictSet */ List *onConflictCols; - /* WHERE for ON CONFLICT UPDATE */ + /* WHERE for ON CONFLICT UPDATE/SELECT */ Node *onConflictWhere; /* RTI of the EXCLUDED pseudo relation */ Index exclRelRTI; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 1b4436f2ff6..34302b42205 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -21,6 +21,7 @@ #include "access/cmptype.h" #include "nodes/bitmapset.h" #include "nodes/pg_list.h" +#include "nodes/lockoptions.h" typedef enum OverridingKind @@ -2363,14 +2364,14 @@ typedef struct FromExpr * * The optimizer requires a list of inference elements, and optionally a WHERE * clause to infer a unique index. The unique index (or, occasionally, - * indexes) inferred are used to arbitrate whether or not the alternative ON - * CONFLICT path is taken. + * indexes) inferred are used to arbitrate whether or not the alternative + * ON CONFLICT path is taken. *---------- */ typedef struct OnConflictExpr { NodeTag type; - OnConflictAction action; /* DO NOTHING or UPDATE? */ + OnConflictAction action; /* DO NOTHING, SELECT, or UPDATE */ /* Arbiter */ List *arbiterElems; /* unique index arbiter list (of @@ -2378,9 +2379,14 @@ typedef struct OnConflictExpr Node *arbiterWhere; /* unique index arbiter WHERE clause */ Oid constraint; /* pg_constraint OID for arbiter */ - /* ON CONFLICT UPDATE */ + /* ON CONFLICT DO SELECT */ + LockClauseStrength lockStrength; /* strength of lock for DO SELECT */ + + /* ON CONFLICT DO UPDATE */ List *onConflictSet; /* List of ON CONFLICT SET TargetEntrys */ - Node *onConflictWhere; /* qualifiers to restrict UPDATE to */ + + /* both ON CONFLICT DO SELECT and UPDATE */ + Node *onConflictWhere; /* qualifiers to restrict SELECT/UPDATE */ int exclRelIndex; /* RT index of 'excluded' relation */ List *exclRelTlist; /* tlist of the EXCLUDED pseudo relation */ } OnConflictExpr; diff --git a/src/test/isolation/expected/insert-conflict-do-select.out b/src/test/isolation/expected/insert-conflict-do-select.out new file mode 100644 index 00000000000..bccfd47dcfb --- /dev/null +++ b/src/test/isolation/expected/insert-conflict-do-select.out @@ -0,0 +1,138 @@ +Parsed test spec with 2 sessions + +starting permutation: insert1 insert2 c1 select2 c2 +step insert1: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step c1: COMMIT; +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; + +starting permutation: insert1_update insert2_update c1 select2 c2 +step insert1_update: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2_update: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +step c1: COMMIT; +step insert2_update: <... completed> +key|val +---+-------- + 1|original +(1 row) + +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; + +starting permutation: insert1_update insert2_update a1 select2 c2 +step insert1_update: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2_update: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +step a1: ABORT; +step insert2_update: <... completed> +key|val +---+-------- + 1|original +(1 row) + +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; + +starting permutation: insert1_keyshare insert2_update c1 select2 c2 +step insert1_keyshare: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2_update: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +step c1: COMMIT; +step insert2_update: <... completed> +key|val +---+-------- + 1|original +(1 row) + +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; + +starting permutation: insert1_share insert2_update c1 select2 c2 +step insert1_share: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR SHARE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2_update: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +step c1: COMMIT; +step insert2_update: <... completed> +key|val +---+-------- + 1|original +(1 row) + +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; + +starting permutation: insert1_nokeyupd insert2_update c1 select2 c2 +step insert1_nokeyupd: INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR NO KEY UPDATE RETURNING *; +key|val +---+-------- + 1|original +(1 row) + +step insert2_update: INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; +step c1: COMMIT; +step insert2_update: <... completed> +key|val +---+-------- + 1|original +(1 row) + +step select2: SELECT * FROM doselect; +key|val +---+-------- + 1|original +(1 row) + +step c2: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 112f05a3677..a4711b83517 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -53,6 +53,7 @@ test: insert-conflict-do-update test: insert-conflict-do-update-2 test: insert-conflict-do-update-3 test: insert-conflict-specconflict +test: insert-conflict-do-select test: merge-insert-update test: merge-delete test: merge-update diff --git a/src/test/isolation/specs/insert-conflict-do-select.spec b/src/test/isolation/specs/insert-conflict-do-select.spec new file mode 100644 index 00000000000..dcfd9f8cb53 --- /dev/null +++ b/src/test/isolation/specs/insert-conflict-do-select.spec @@ -0,0 +1,53 @@ +# INSERT...ON CONFLICT DO SELECT test +# +# This test verifies locking behavior of ON CONFLICT DO SELECT with different +# lock strengths: no lock, FOR KEY SHARE, FOR SHARE, FOR NO KEY UPDATE, and +# FOR UPDATE. + +setup +{ + CREATE TABLE doselect (key int primary key, val text); + INSERT INTO doselect VALUES (1, 'original'); +} + +teardown +{ + DROP TABLE doselect; +} + +session s1 +setup +{ + BEGIN ISOLATION LEVEL READ COMMITTED; +} +step insert1 { INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT RETURNING *; } +step insert1_keyshare { INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR KEY SHARE RETURNING *; } +step insert1_share { INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR SHARE RETURNING *; } +step insert1_nokeyupd { INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR NO KEY UPDATE RETURNING *; } +step insert1_update { INSERT INTO doselect(key, val) VALUES(1, 'insert1') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; } +step c1 { COMMIT; } +step a1 { ABORT; } + +session s2 +setup +{ + BEGIN ISOLATION LEVEL READ COMMITTED; +} +step insert2 { INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT RETURNING *; } +step insert2_update { INSERT INTO doselect(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING *; } +step select2 { SELECT * FROM doselect; } +step c2 { COMMIT; } + +# Test 1: DO SELECT without locking - should not block +permutation insert1 insert2 c1 select2 c2 + +# Test 2: DO SELECT FOR UPDATE - should block until first transaction commits +permutation insert1_update insert2_update c1 select2 c2 + +# Test 3: DO SELECT FOR UPDATE - should unblock when first transaction aborts +permutation insert1_update insert2_update a1 select2 c2 + +# Test 4: Different lock strengths all properly acquire locks +permutation insert1_keyshare insert2_update c1 select2 c2 +permutation insert1_share insert2_update c1 select2 c2 +permutation insert1_nokeyupd insert2_update c1 select2 c2 diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index 1bbf59cca02..8bc1f0cd5ab 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -776,10 +776,16 @@ DETAIL: Key (c1, (c2::circle))=(<(20,20),10>, <(0,0),4>) conflicts with existin -- succeed, because violation is ignored INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO NOTHING; --- fail, because DO UPDATE variant requires unique index +-- fail, because DO UPDATE variant requires unique index. +-- (without a unique index, we can't know which row to update) INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO UPDATE SET c2 = EXCLUDED.c2; ERROR: ON CONFLICT DO UPDATE not supported with exclusion constraints +-- fail, just like DO UPDATE. +-- otherwise, we could return multiple rows which seems odd, if not exactly wrong +INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') + ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO SELECT RETURNING *; +ERROR: ON CONFLICT DO SELECT not supported with exclusion constraints -- succeed because c1 doesn't overlap INSERT INTO circles VALUES('<(20,20), 1>', '<(0,0), 5>'); -- succeed because c2 doesn't overlap diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index db668474684..839e33883a0 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -249,6 +249,169 @@ insert into insertconflicttest values (2, 'Orange') on conflict (key, key, key) insert into insertconflicttest values (1, 'Apple'), (2, 'Orange') on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key); +----- INSERT ON CONFLICT DO SELECT PERMISSION TESTS --- +create table conflictselect_perv(key int4, fruit text); +create unique index x_idx on conflictselect_perv(key); +create role regress_conflict_alice; +grant all on schema public to regress_conflict_alice; +grant insert on conflictselect_perv to regress_conflict_alice; +grant select(key) on conflictselect_perv to regress_conflict_alice; +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.key = 1 returning 1; --ok + ?column? +---------- + 1 +(1 row) + +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning 1; --fail +ERROR: permission denied for table conflictselect_perv +reset role; +grant select(fruit) on conflictselect_perv to regress_conflict_alice; +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning 1; --ok + ?column? +---------- + 1 +(1 row) + +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning 1; --fail +ERROR: permission denied for table conflictselect_perv +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for no key update where i.fruit = 'Apple' returning 1; --fail +ERROR: permission denied for table conflictselect_perv +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for share where i.fruit = 'Apple' returning 1; --fail +ERROR: permission denied for table conflictselect_perv +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for key share where i.fruit = 'Apple' returning 1; --fail +ERROR: permission denied for table conflictselect_perv +reset role; +grant update (fruit) on conflictselect_perv to regress_conflict_alice; +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for no key update where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for share where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for key share where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +reset role; +drop table conflictselect_perv; +revoke all on schema public from regress_conflict_alice; +drop role regress_conflict_alice; +------- END OF PERMISSION TESTS ------------ +-- DO SELECT +delete from insertconflicttest where fruit = 'Apple'; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select; -- fails +ERROR: ON CONFLICT DO SELECT requires a RETURNING clause +LINE 1: ...nsert into insertconflicttest values (1, 'Apple') on conflic... + ^ +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Orange' returning *; + key | fruit +-----+------- +(0 rows) + +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Apple' returning *; + key | fruit +-----+------- +(0 rows) + +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Orange' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +delete from insertconflicttest where fruit = 'Apple'; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Orange' returning *; + key | fruit +-----+------- +(0 rows) + +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Apple' returning *; + key | fruit +-----+------- +(0 rows) + +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Orange' returning *; + key | fruit +-----+------- + 1 | Apple +(1 row) + +insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*; + key | fruit | key | fruit | key | fruit +-----+-------+-----+-------+-----+------- + | | 3 | Pear | 3 | Pear +(1 row) + +insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*; + key | fruit | key | fruit | key | fruit +-----+-------+-----+-------+-----+------- + 3 | Pear | 3 | Pear | 3 | Pear +(1 row) + +explain (costs off) insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for key share returning *; + QUERY PLAN +--------------------------------------------- + Insert on insertconflicttest + Conflict Resolution: SELECT FOR KEY SHARE + Conflict Arbiter Indexes: key_index + -> 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; @@ -735,13 +898,58 @@ insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 = ERROR: ON CONFLICT DO UPDATE command cannot affect row a second time HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values. commit; +begin transaction isolation level read committed; +insert into selfconflict values (7,1), (7,2) on conflict(f1) do select returning *; + f1 | f2 +----+---- + 7 | 1 + 7 | 1 +(2 rows) + +commit; +begin transaction isolation level repeatable read; +insert into selfconflict values (8,1), (8,2) on conflict(f1) do select returning *; + f1 | f2 +----+---- + 8 | 1 + 8 | 1 +(2 rows) + +commit; +begin transaction isolation level serializable; +insert into selfconflict values (9,1), (9,2) on conflict(f1) do select returning *; + f1 | f2 +----+---- + 9 | 1 + 9 | 1 +(2 rows) + +commit; +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; +begin transaction isolation level repeatable read; +insert into selfconflict values (11,1), (11,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; +begin transaction isolation level serializable; +insert into selfconflict values (12,1), (12,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; select * from selfconflict; f1 | f2 ----+---- 1 | 1 2 | 1 3 | 1 -(3 rows) + 7 | 1 + 8 | 1 + 9 | 1 +(6 rows) drop table selfconflict; -- check ON CONFLICT handling with partitioned tables @@ -752,11 +960,31 @@ insert into parted_conflict_test values (1, 'a') on conflict do nothing; -- index on a required, which does exist in parent insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing; insert into parted_conflict_test values (1, 'a') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test values (1, 'a') on conflict (a) do select returning *; + a | b +---+--- + 1 | a +(1 row) + +insert into parted_conflict_test values (1, 'a') on conflict (a) do select for update returning *; + a | b +---+--- + 1 | a +(1 row) + -- targeting partition directly will work insert into parted_conflict_test_1 values (1, 'a') on conflict (a) do nothing; insert into parted_conflict_test_1 values (1, 'b') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test_1 values (1, 'b') on conflict (a) do select returning b; + b +--- + b +(1 row) + -- index on b required, which doesn't exist in parent -insert into parted_conflict_test values (2, 'b') on conflict (b) do update set a = excluded.a; +insert into parted_conflict_test values (2, 'b') on conflict (b) do update set a = excluded.a; -- fail +ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification +insert into parted_conflict_test values (2, 'b') on conflict (b) do select returning b; -- fail ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification -- targeting partition directly will work insert into parted_conflict_test_1 values (2, 'b') on conflict (b) do update set a = excluded.a; @@ -767,13 +995,31 @@ select * from parted_conflict_test order by a; 2 | b (1 row) --- now check that DO UPDATE works correctly for target partition with +-- now check that DO UPDATE/SELECT works correctly for target partition with -- different attribute numbers create table parted_conflict_test_2 (b char, a int unique); alter table parted_conflict_test attach partition parted_conflict_test_2 for values in (3); truncate parted_conflict_test; insert into parted_conflict_test values (3, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test values (3, 'b') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test values (3, 'a') on conflict (a) do select returning b; + b +--- + b +(1 row) + +insert into parted_conflict_test values (3, 'a') on conflict (a) do select where excluded.b = 'a' returning parted_conflict_test; + parted_conflict_test +---------------------- + (3,b) +(1 row) + +insert into parted_conflict_test values (3, 'a') on conflict (a) do select where parted_conflict_test.b = 'b' returning b; + b +--- + b +(1 row) + -- should see (3, 'b') select * from parted_conflict_test order by a; a | b @@ -787,6 +1033,12 @@ create table parted_conflict_test_3 partition of parted_conflict_test for values truncate parted_conflict_test; insert into parted_conflict_test (a, b) values (4, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test (a, b) values (4, 'b') on conflict (a) do update set b = excluded.b where parted_conflict_test.b = 'a'; +insert into parted_conflict_test (a, b) values (4, 'b') on conflict (a) do select returning b; + b +--- + b +(1 row) + -- should see (4, 'b') select * from parted_conflict_test order by a; a | b @@ -800,6 +1052,11 @@ create table parted_conflict_test_4_1 partition of parted_conflict_test_4 for va truncate parted_conflict_test; insert into parted_conflict_test (a, b) values (5, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test (a, b) values (5, 'b') on conflict (a) do update set b = excluded.b where parted_conflict_test.b = 'a'; +insert into parted_conflict_test (a, b) values (5, 'b') on conflict (a) do select where parted_conflict_test.b = 'a' returning b; + b +--- +(0 rows) + -- should see (5, 'b') select * from parted_conflict_test order by a; a | b @@ -820,6 +1077,58 @@ select * from parted_conflict_test order by a; 4 | b (3 rows) +-- 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 +(3 rows) + +-- should see original values (1, 'a'), (2, 'b'), (4, 'c') +select * from parted_conflict_test order by a; + a | b +---+--- + 1 | a + 2 | b + 4 | c +(3 rows) + +-- test DO SELECT with WHERE filtering across partitions +insert into parted_conflict_test (a, b) values (1, 'n') on conflict (a) do select where parted_conflict_test.b = 'a' returning *; + a | b +---+--- + 1 | a +(1 row) + +insert into parted_conflict_test (a, b) values (2, 'n') on conflict (a) do select where parted_conflict_test.b = 'x' returning *; + a | b +---+--- +(0 rows) + +-- test DO SELECT with EXCLUDED in WHERE across partitions with different layouts +insert into parted_conflict_test (a, b) values (3, 't') on conflict (a) do select where excluded.b = 't' returning *; + a | b +---+--- + 3 | t +(1 row) + +-- test DO SELECT FOR UPDATE across different partition layouts +insert into parted_conflict_test (a, b) values (1, 'l') on conflict (a) do select for update returning *; + a | b +---+--- + 1 | a +(1 row) + +insert into parted_conflict_test (a, b) values (3, 'l') on conflict (a) do select for update returning *; + a | b +---+--- + 3 | t +(1 row) + drop table parted_conflict_test; -- test behavior of inserting a conflicting tuple into an intermediate -- partitioning level diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index c958ef4d70a..d6a2be1f96e 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -217,6 +217,48 @@ NOTICE: SELECT USING on rls_test_tgt.(3,"tgt d","TGT D") 3 | tgt d | TGT D (1 row) +ROLLBACK; +-- ON CONFLICT DO SELECT should be similar to DO UPDATE, except there +-- is not need to check the UPDATE policy in that case. +BEGIN; +INSERT INTO rls_test_tgt VALUES (4, 'tgt a') ON CONFLICT (a) DO SELECT RETURNING *; +NOTICE: INSERT CHECK on rls_test_tgt.(4,"tgt a","TGT A") +NOTICE: SELECT USING on rls_test_tgt.(4,"tgt a","TGT A") + a | b | c +---+-------+------- + 4 | tgt a | TGT A +(1 row) + +INSERT INTO rls_test_tgt VALUES (4, 'tgt a') ON CONFLICT (a) DO SELECT RETURNING *; +NOTICE: INSERT CHECK on rls_test_tgt.(4,"tgt a","TGT A") +NOTICE: SELECT USING on rls_test_tgt.(4,"tgt a","TGT A") +NOTICE: SELECT USING on rls_test_tgt.(4,"tgt a","TGT A") + a | b | c +---+-------+------- + 4 | tgt a | TGT A +(1 row) + +ROLLBACK; +-- ON CONFLICT DO SELECT FOR UPDATE should have the exact same RLS behaviour as DO UPDATE +BEGIN; +INSERT INTO rls_test_tgt VALUES (5, 'tgt a') ON CONFLICT (a) DO SELECT FOR UPDATE RETURNING *; +NOTICE: INSERT CHECK on rls_test_tgt.(5,"tgt a","TGT A") +NOTICE: SELECT USING on rls_test_tgt.(5,"tgt a","TGT A") + a | b | c +---+-------+------- + 5 | tgt a | TGT A +(1 row) + +INSERT INTO rls_test_tgt VALUES (5, 'tgt c') ON CONFLICT (a) DO SELECT FOR UPDATE RETURNING *; +NOTICE: INSERT CHECK on rls_test_tgt.(5,"tgt c","TGT C") +NOTICE: SELECT USING on rls_test_tgt.(5,"tgt c","TGT C") +NOTICE: UPDATE USING on rls_test_tgt.(5,"tgt a","TGT A") +NOTICE: SELECT USING on rls_test_tgt.(5,"tgt a","TGT A") + a | b | c +---+-------+------- + 5 | tgt a | TGT A +(1 row) + ROLLBACK; -- MERGE should always apply SELECT USING policy clauses to both source and -- target rows @@ -2394,10 +2436,58 @@ INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel') ON CONFLICT (did) DO UPDATE SET dauthor = 'regress_rls_carol'; ERROR: new row violates row-level security policy for table "document" -- +-- 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; +-- DO SELECT requires SELECT rights, should succeed for novel +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; + did | dauthor | dtitle +-----+-----------------+---------------- + 1 | regress_rls_bob | my first novel +(1 row) + +-- DO SELECT requires SELECT rights, should fail for non-novel +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') + ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; +ERROR: new row violates row-level security policy for table "document" +-- DO SELECT with WHERE and EXCLUDED reference +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT WHERE excluded.dlevel = 1 RETURNING did, dauthor, dtitle; + did | dauthor | dtitle +-----+-----------------+---------------- + 1 | regress_rls_bob | my first novel +(1 row) + +-- DO SELECT FOR UPDATE requires both SELECT and UPDATE rights, should succeed for novel +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT FOR UPDATE RETURNING did, dauthor, dtitle; + did | dauthor | dtitle +-----+-----------------+---------------- + 1 | regress_rls_bob | my first novel +(1 row) + +-- DO SELECT FOR UPDATE requires UPDATE rights, should fail for non-novel +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') + ON CONFLICT (did) DO SELECT FOR UPDATE RETURNING did, dauthor, dtitle; +ERROR: new row violates row-level security policy for table "document" +SET SESSION AUTHORIZATION regress_rls_alice; +DROP POLICY p1_select_novels ON document; +DROP POLICY p2_insert_own ON document; +DROP POLICY p3_update_novels ON document; +-- -- MERGE -- RESET SESSION AUTHORIZATION; -DROP POLICY p3_with_all ON document; ALTER TABLE document ADD COLUMN dnotes text DEFAULT ''; -- all documents are readable CREATE POLICY p1 ON document FOR SELECT USING (true); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index c337f0bc30d..27cedd0d19b 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -3564,6 +3564,61 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name; (3 rows) DROP RULE hat_upsert ON hats; +-- DO SELECT with a WHERE clause +CREATE RULE hat_confsel AS ON INSERT TO hats + DO INSTEAD + INSERT INTO hat_data VALUES ( + NEW.hat_name, + NEW.hat_color) + ON CONFLICT (hat_name) + DO SELECT FOR UPDATE + WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.* + RETURNING *; +SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename; + definition +-------------------------------------------------------------------------------------- + CREATE RULE hat_confsel AS + + ON INSERT TO public.hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) + + VALUES (new.hat_name, new.hat_color) ON CONFLICT(hat_name) DO SELECT FOR UPDATE + + WHERE ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*))+ + RETURNING hat_data.hat_name, + + hat_data.hat_color; +(1 row) + +-- fails without RETURNING +INSERT INTO hats VALUES ('h7', 'blue'); +ERROR: ON CONFLICT DO SELECT requires a RETURNING clause +DETAIL: A rule action is INSERT ... ON CONFLICT DO SELECT, which requires a RETURNING clause. +-- works (returns conflicts) +EXPLAIN (costs off) +INSERT INTO hats VALUES ('h7', 'blue') RETURNING *; + QUERY PLAN +------------------------------------------------------------------------------------------------- + Insert on hat_data + Conflict Resolution: SELECT FOR UPDATE + Conflict Arbiter Indexes: hat_data_unique_idx + Conflict Filter: ((excluded.hat_color <> 'forbidden'::bpchar) AND (hat_data.* <> excluded.*)) + -> Result +(5 rows) + +INSERT INTO hats VALUES ('h7', 'blue') RETURNING *; + hat_name | hat_color +------------+------------ + h7 | black +(1 row) + +-- conflicts excluded by WHERE clause +INSERT INTO hats VALUES ('h7', 'forbidden') RETURNING *; + hat_name | hat_color +----------+----------- +(0 rows) + +INSERT INTO hats VALUES ('h7', 'black') RETURNING *; + hat_name | hat_color +----------+----------- +(0 rows) + +DROP RULE hat_confsel ON hats; drop table hats; drop table hat_data; -- test for pg_get_functiondef properly regurgitating SET parameters diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 03df7e75b7b..a3c811effc8 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -316,6 +316,37 @@ SELECT * FROM rw_view15; 3 | UNSPECIFIED (6 rows) +-- 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 + 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) + 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 + 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) + 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 + a | upper +---+------- +(0 rows) + SELECT * FROM rw_view15; a | upper ----+------------- diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 733a1dbccfe..b093e92850f 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -565,9 +565,14 @@ INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>'); -- succeed, because violation is ignored INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO NOTHING; --- fail, because DO UPDATE variant requires unique index +-- fail, because DO UPDATE variant requires unique index. +-- (without a unique index, we can't know which row to update) INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO UPDATE SET c2 = EXCLUDED.c2; +-- fail, just like DO UPDATE. +-- otherwise, we could return multiple rows which seems odd, if not exactly wrong +INSERT INTO circles VALUES('<(20,20), 10>', '<(0,0), 4>') + ON CONFLICT ON CONSTRAINT circles_c1_c2_excl DO SELECT RETURNING *; -- succeed because c1 doesn't overlap INSERT INTO circles VALUES('<(20,20), 1>', '<(0,0), 5>'); -- succeed because c2 doesn't overlap diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index 549c46452ec..8f856cdb245 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -101,6 +101,65 @@ insert into insertconflicttest values (1, 'Apple'), (2, 'Orange') on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key); +----- INSERT ON CONFLICT DO SELECT PERMISSION TESTS --- +create table conflictselect_perv(key int4, fruit text); +create unique index x_idx on conflictselect_perv(key); +create role regress_conflict_alice; +grant all on schema public to regress_conflict_alice; +grant insert on conflictselect_perv to regress_conflict_alice; +grant select(key) on conflictselect_perv to regress_conflict_alice; + +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.key = 1 returning 1; --ok +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning 1; --fail + +reset role; +grant select(fruit) on conflictselect_perv to regress_conflict_alice; +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning 1; --ok +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning 1; --fail +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for no key update where i.fruit = 'Apple' returning 1; --fail +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for share where i.fruit = 'Apple' returning 1; --fail +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for key share where i.fruit = 'Apple' returning 1; --fail + +reset role; +grant update (fruit) on conflictselect_perv to regress_conflict_alice; +set role regress_conflict_alice; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for no key update where i.fruit = 'Apple' returning *; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for share where i.fruit = 'Apple' returning *; +insert into conflictselect_perv as i values (1, 'Apple') on conflict (key) do select for key share where i.fruit = 'Apple' returning *; + +reset role; +drop table conflictselect_perv; +revoke all on schema public from regress_conflict_alice; +drop role regress_conflict_alice; +------- END OF PERMISSION TESTS ------------ + +-- DO SELECT +delete from insertconflicttest where fruit = 'Apple'; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select; -- fails +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select returning *; +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Apple' returning *; +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select where i.fruit = 'Orange' returning *; +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Apple' returning *; +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select where excluded.fruit = 'Orange' returning *; +delete from insertconflicttest where fruit = 'Apple'; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *; +insert into insertconflicttest values (1, 'Apple') on conflict (key) do select for update returning *; +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Apple' returning *; +insert into insertconflicttest as i values (1, 'Apple') on conflict (key) do select for update where i.fruit = 'Orange' returning *; +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Apple' returning *; +insert into insertconflicttest as i values (1, 'Orange') on conflict (key) do select for update where excluded.fruit = 'Orange' returning *; +insert into insertconflicttest as ict values (3, 'Pear') on conflict (key) do select for update returning old.*, new.*, ict.*; +insert into insertconflicttest as ict values (3, 'Banana') on conflict (key) do select for update returning old.*, new.*, ict.*; + +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; @@ -454,6 +513,30 @@ begin transaction isolation level serializable; insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 = 0; commit; +begin transaction isolation level read committed; +insert into selfconflict values (7,1), (7,2) on conflict(f1) do select returning *; +commit; + +begin transaction isolation level repeatable read; +insert into selfconflict values (8,1), (8,2) on conflict(f1) do select returning *; +commit; + +begin transaction isolation level serializable; +insert into selfconflict values (9,1), (9,2) on conflict(f1) do select returning *; +commit; + +begin transaction isolation level read committed; +insert into selfconflict values (10,1), (10,2) on conflict(f1) do select for update returning *; +commit; + +begin transaction isolation level repeatable read; +insert into selfconflict values (11,1), (11,2) on conflict(f1) do select for update returning *; +commit; + +begin transaction isolation level serializable; +insert into selfconflict values (12,1), (12,2) on conflict(f1) do select for update returning *; +commit; + select * from selfconflict; drop table selfconflict; @@ -468,13 +551,17 @@ insert into parted_conflict_test values (1, 'a') on conflict do nothing; -- index on a required, which does exist in parent insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing; insert into parted_conflict_test values (1, 'a') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test values (1, 'a') on conflict (a) do select returning *; +insert into parted_conflict_test values (1, 'a') on conflict (a) do select for update returning *; -- targeting partition directly will work insert into parted_conflict_test_1 values (1, 'a') on conflict (a) do nothing; insert into parted_conflict_test_1 values (1, 'b') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test_1 values (1, 'b') on conflict (a) do select returning b; -- index on b required, which doesn't exist in parent -insert into parted_conflict_test values (2, 'b') on conflict (b) do update set a = excluded.a; +insert into parted_conflict_test values (2, 'b') on conflict (b) do update set a = excluded.a; -- fail +insert into parted_conflict_test values (2, 'b') on conflict (b) do select returning b; -- fail -- targeting partition directly will work insert into parted_conflict_test_1 values (2, 'b') on conflict (b) do update set a = excluded.a; @@ -482,13 +569,16 @@ insert into parted_conflict_test_1 values (2, 'b') on conflict (b) do update set -- should see (2, 'b') select * from parted_conflict_test order by a; --- now check that DO UPDATE works correctly for target partition with +-- now check that DO UPDATE/SELECT works correctly for target partition with -- different attribute numbers create table parted_conflict_test_2 (b char, a int unique); alter table parted_conflict_test attach partition parted_conflict_test_2 for values in (3); truncate parted_conflict_test; insert into parted_conflict_test values (3, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test values (3, 'b') on conflict (a) do update set b = excluded.b; +insert into parted_conflict_test values (3, 'a') on conflict (a) do select returning b; +insert into parted_conflict_test values (3, 'a') on conflict (a) do select where excluded.b = 'a' returning parted_conflict_test; +insert into parted_conflict_test values (3, 'a') on conflict (a) do select where parted_conflict_test.b = 'b' returning b; -- should see (3, 'b') select * from parted_conflict_test order by a; @@ -499,6 +589,7 @@ create table parted_conflict_test_3 partition of parted_conflict_test for values truncate parted_conflict_test; insert into parted_conflict_test (a, b) values (4, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test (a, b) values (4, 'b') on conflict (a) do update set b = excluded.b where parted_conflict_test.b = 'a'; +insert into parted_conflict_test (a, b) values (4, 'b') on conflict (a) do select returning b; -- should see (4, 'b') select * from parted_conflict_test order by a; @@ -509,6 +600,7 @@ create table parted_conflict_test_4_1 partition of parted_conflict_test_4 for va truncate parted_conflict_test; insert into parted_conflict_test (a, b) values (5, 'a') on conflict (a) do update set b = excluded.b; insert into parted_conflict_test (a, b) values (5, 'b') on conflict (a) do update set b = excluded.b where parted_conflict_test.b = 'a'; +insert into parted_conflict_test (a, b) values (5, 'b') on conflict (a) do select where parted_conflict_test.b = 'a' returning b; -- should see (5, 'b') select * from parted_conflict_test order by a; @@ -521,6 +613,25 @@ insert into parted_conflict_test (a, b) values (1, 'b'), (2, 'c'), (4, 'b') on c -- should see (1, 'b'), (2, 'a'), (4, 'b') 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 *; + +-- should see original values (1, 'a'), (2, 'b'), (4, 'c') +select * from parted_conflict_test order by a; + +-- test DO SELECT with WHERE filtering across partitions +insert into parted_conflict_test (a, b) values (1, 'n') on conflict (a) do select where parted_conflict_test.b = 'a' returning *; +insert into parted_conflict_test (a, b) values (2, 'n') on conflict (a) do select where parted_conflict_test.b = 'x' returning *; + +-- test DO SELECT with EXCLUDED in WHERE across partitions with different layouts +insert into parted_conflict_test (a, b) values (3, 't') on conflict (a) do select where excluded.b = 't' returning *; + +-- test DO SELECT FOR UPDATE across different partition layouts +insert into parted_conflict_test (a, b) values (1, 'l') on conflict (a) do select for update returning *; +insert into parted_conflict_test (a, b) values (3, 'l') on conflict (a) do select for update returning *; + drop table parted_conflict_test; -- test behavior of inserting a conflicting tuple into an intermediate diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 5d923c5ca3b..b3e282c19d3 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -141,6 +141,19 @@ INSERT INTO rls_test_tgt VALUES (3, 'tgt a') ON CONFLICT (a) DO UPDATE SET b = ' INSERT INTO rls_test_tgt VALUES (3, 'tgt c') ON CONFLICT (a) DO UPDATE SET b = 'tgt d' RETURNING *; ROLLBACK; +-- ON CONFLICT DO SELECT should be similar to DO UPDATE, except there +-- is not need to check the UPDATE policy in that case. +BEGIN; +INSERT INTO rls_test_tgt VALUES (4, 'tgt a') ON CONFLICT (a) DO SELECT RETURNING *; +INSERT INTO rls_test_tgt VALUES (4, 'tgt a') ON CONFLICT (a) DO SELECT RETURNING *; +ROLLBACK; + +-- ON CONFLICT DO SELECT FOR UPDATE should have the exact same RLS behaviour as DO UPDATE +BEGIN; +INSERT INTO rls_test_tgt VALUES (5, 'tgt a') ON CONFLICT (a) DO SELECT FOR UPDATE RETURNING *; +INSERT INTO rls_test_tgt VALUES (5, 'tgt c') ON CONFLICT (a) DO SELECT FOR UPDATE RETURNING *; +ROLLBACK; + -- MERGE should always apply SELECT USING policy clauses to both source and -- target rows MERGE INTO rls_test_tgt t USING rls_test_src s ON t.a = s.a @@ -952,11 +965,53 @@ INSERT INTO document VALUES (4, (SELECT cid from category WHERE cname = 'novel') INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') ON CONFLICT (did) DO UPDATE SET dauthor = 'regress_rls_carol'; +-- +-- 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; + +-- DO SELECT requires SELECT rights, should succeed for novel +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; + +-- DO SELECT requires SELECT rights, should fail for non-novel +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') + ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; + +-- DO SELECT with WHERE and EXCLUDED reference +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT WHERE excluded.dlevel = 1 RETURNING did, dauthor, dtitle; + +-- DO SELECT FOR UPDATE requires both SELECT and UPDATE rights, should succeed for novel +INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') + ON CONFLICT (did) DO SELECT FOR UPDATE RETURNING did, dauthor, dtitle; + +-- DO SELECT FOR UPDATE requires UPDATE rights, should fail for non-novel +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') + ON CONFLICT (did) DO SELECT FOR UPDATE RETURNING did, dauthor, dtitle; + +SET SESSION AUTHORIZATION regress_rls_alice; +DROP POLICY p1_select_novels ON document; +DROP POLICY p2_insert_own ON document; +DROP POLICY p3_update_novels ON document; + + -- -- MERGE -- RESET SESSION AUTHORIZATION; -DROP POLICY p3_with_all ON document; ALTER TABLE document ADD COLUMN dnotes text DEFAULT ''; -- all documents are readable diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 3f240bec7b0..40f5c16e540 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1205,6 +1205,32 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name; DROP RULE hat_upsert ON hats; +-- DO SELECT with a WHERE clause +CREATE RULE hat_confsel AS ON INSERT TO hats + DO INSTEAD + INSERT INTO hat_data VALUES ( + NEW.hat_name, + NEW.hat_color) + ON CONFLICT (hat_name) + DO SELECT FOR UPDATE + WHERE excluded.hat_color <> 'forbidden' AND hat_data.* != excluded.* + RETURNING *; +SELECT definition FROM pg_rules WHERE tablename = 'hats' ORDER BY rulename; + +-- fails without RETURNING +INSERT INTO hats VALUES ('h7', 'blue'); + +-- works (returns conflicts) +EXPLAIN (costs off) +INSERT INTO hats VALUES ('h7', 'blue') RETURNING *; +INSERT INTO hats VALUES ('h7', 'blue') RETURNING *; + +-- conflicts excluded by WHERE clause +INSERT INTO hats VALUES ('h7', 'forbidden') RETURNING *; +INSERT INTO hats VALUES ('h7', 'black') RETURNING *; + +DROP RULE hat_confsel ON hats; + drop table hats; drop table hat_data; diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql index c071fffc116..d9f1ca5bd97 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -106,6 +106,14 @@ INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE set a = excluded. SELECT * FROM rw_view15; INSERT INTO rw_view15 (a) VALUES (3) ON CONFLICT (a) DO UPDATE set upper = 'blarg'; -- fails 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 SELECT * FROM rw_view15; ALTER VIEW rw_view15 ALTER COLUMN upper SET DEFAULT 'NOT SET'; INSERT INTO rw_view15 (a) VALUES (4); -- should fail diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 57a8f0366a5..c5c0d2a1c2c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1816,9 +1816,9 @@ OldToNewMappingData OnCommitAction OnCommitItem OnConflictAction +OnConflictActionState OnConflictClause OnConflictExpr -OnConflictSetState OpClassCacheEnt OpExpr OpFamilyMember -- 2.48.1