From f8ae995d459c91c9d62312117f7ad0cde33f248c Mon Sep 17 00:00:00 2001 From: Viktor Holmberg Date: Thu, 4 Sep 2025 21:22:45 +0200 Subject: [PATCH v8 3/3] Remaning fixes for ON CONFLICT DO SELECT --- doc/src/sgml/dml.sgml | 3 +- doc/src/sgml/ref/insert.sgml | 89 +++++++++-- src/backend/executor/execPartition.c | 74 +++++++++- src/backend/executor/nodeModifyTable.c | 6 +- src/include/nodes/execnodes.h | 14 +- src/include/nodes/parsenodes.h | 2 +- src/include/nodes/primnodes.h | 2 +- .../expected/insert-conflict-do-select.out | 138 ++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/insert-conflict-do-select.spec | 53 +++++++ src/test/regress/expected/insert_conflict.out | 91 +++++++++++- src/test/regress/expected/rowsecurity.out | 50 ++++++- src/test/regress/sql/insert_conflict.sql | 28 +++- src/test/regress/sql/rowsecurity.sql | 44 +++++- src/tools/pgindent/typedefs.list | 2 +- 15 files changed, 565 insertions(+), 32 deletions(-) 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/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/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml index 76117c684c5..9b5cd82be70 100644 --- a/doc/src/sgml/ref/insert.sgml +++ b/doc/src/sgml/ref/insert.sgml @@ -37,7 +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 } ] + 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 ) @@ -113,7 +113,8 @@ INSERT INTO table_name [ AS 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. @@ -125,6 +126,9 @@ INSERT INTO table_name [ AS SELECT privilege on any column whose values are read in the ON CONFLICT DO UPDATE expressions or condition. + For ON CONFLICT DO SELECT, SELECT + privilege is required on any column whose values are read in the + condition. @@ -348,7 +352,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. @@ -384,6 +391,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. @@ -415,6 +425,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 @@ -428,7 +445,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. @@ -440,10 +458,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 @@ -452,6 +471,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 @@ -554,12 +585,14 @@ 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. @@ -623,7 +656,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 @@ -809,6 +842,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/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index aa12e9ad2ea..a8f7d1dc5bd 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -735,7 +735,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, */ if (node->onConflictAction == ONCONFLICT_UPDATE) { - OnConflictSetState *onconfl = makeNode(OnConflictSetState); + OnConflictActionState *onconfl = makeNode(OnConflictActionState); TupleConversionMap *map; map = ExecGetRootToChildMap(leaf_part_rri, estate); @@ -859,6 +859,78 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, } } } + else if (node->onConflictAction == ONCONFLICT_SELECT) + { + OnConflictActionState *onconfl = makeNode(OnConflictActionState); + TupleConversionMap *map; + + map = ExecGetRootToChildMap(leaf_part_rri, estate); + Assert(rootResultRelInfo->ri_onConflict != NULL); + + leaf_part_rri->ri_onConflict = onconfl; + + onconfl->oc_LockingStrength = + rootResultRelInfo->ri_onConflict->oc_LockingStrength; + + /* + * Need a separate existing slot for each partition, as the + * partition could be of a different AM, even if the tuple + * descriptors match. + */ + onconfl->oc_Existing = + table_slot_create(leaf_part_rri->ri_RelationDesc, + &mtstate->ps.state->es_tupleTable); + + /* + * If the partition's tuple descriptor matches exactly the root + * parent (the common case), we can re-use the parent's ON + * CONFLICT DO SELECT state. Otherwise, we need to remap the + * WHERE clause for this partition's layout. + */ + if (map == NULL) + { + /* + * 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 WHERE clause + * doesn't store state / is independent of the underlying + * storage. + */ + onconfl->oc_WhereClause = + rootResultRelInfo->ri_onConflict->oc_WhereClause; + } + else if (node->onConflictWhere) + { + /* + * Map the WHERE clause, if it exists. + */ + 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, + INNER_VAR, 0, + part_attmap, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + clause = (List *) + map_variable_attnos((Node *) clause, + firstVarno, 0, + part_attmap, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + onconfl->oc_WhereClause = + ExecInitQual(clause, &mtstate->ps); + } + } } /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 80e2650366c..54a9d8920c5 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2997,7 +2997,7 @@ ExecOnConflictUpdate(ModifyTableContext *context, * speculative insertion. If a qual originating from ON CONFLICT DO UPDATE is * satisfied, select the row. * - * Returns true if if we're done (with or without a select), or false if the + * Returns true if we're done (with or without a select), or false if the * caller must retry the INSERT from scratch. */ static bool @@ -5201,7 +5201,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) */ if (node->onConflictAction == ONCONFLICT_UPDATE) { - OnConflictSetState *onconfl = makeNode(OnConflictSetState); + OnConflictActionState *onconfl = makeNode(OnConflictActionState); ExprContext *econtext; TupleDesc relationDesc; @@ -5252,7 +5252,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } else if (node->onConflictAction == ONCONFLICT_SELECT) { - OnConflictSetState *onconfl = makeNode(OnConflictSetState); + OnConflictActionState *onconfl = makeNode(OnConflictActionState); /* already exists if created by RETURNING processing above */ if (mtstate->ps.ps_ExprContext == NULL) diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 727807abed7..297969efad3 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -422,21 +422,21 @@ 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_LockingStrength; /* strengh of lock for ON CONFLICT - * DO SELECT, or LCS_NONE */ + LockClauseStrength oc_LockingStrength; /* strength of lock for ON + * CONFLICT DO SELECT, or LCS_NONE */ ExprState *oc_WhereClause; /* state for the WHERE clause */ -} OnConflictSetState; +} OnConflictActionState; /* ---------------- * MergeActionState information @@ -582,7 +582,7 @@ typedef struct ResultRelInfo List *ri_onConflictArbiterIndexes; /* ON CONFLICT evaluation state */ - OnConflictSetState *ri_onConflict; + OnConflictActionState *ri_onConflict; /* for MERGE, lists of MergeActionState (one per MergeMatchKind) */ List *ri_MergeActions[NUM_MERGE_MATCH_KINDS]; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 03cd0638750..31c73abe87b 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1655,7 +1655,7 @@ typedef struct OnConflictClause OnConflictAction action; /* DO NOTHING, SELECT or UPDATE? */ InferClause *infer; /* Optional index inference clause */ List *targetList; /* the target list (of ResTarget) */ - LockClauseStrength lockingStrength; /* strengh of lock for DO SELECT, or + LockClauseStrength lockingStrength; /* strength of lock for DO SELECT, or * LCS_NONE */ Node *whereClause; /* qualifications */ ParseLoc location; /* token location, or -1 if unknown */ diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 0af96f1bf15..d87686de000 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -2383,7 +2383,7 @@ typedef struct OnConflictExpr Node *onConflictWhere; /* qualifiers to restrict SELECT/UPDATE to */ /* ON CONFLICT SELECT */ - LockClauseStrength lockingStrength; /* strengh of lock for DO SELECT, or + LockClauseStrength lockingStrength; /* strength of lock for DO SELECT, or * LCS_NONE */ /* ON CONFLICT UPDATE */ 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 5afae33d370..e30dc7609cb 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -54,6 +54,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/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index d226c472340..8a4d6f540df 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -893,11 +893,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; @@ -915,6 +935,12 @@ alter table parted_conflict_test attach partition parted_conflict_test_2 for val 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) + -- should see (3, 'b') select * from parted_conflict_test order by a; a | b @@ -928,6 +954,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 @@ -941,6 +973,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 @@ -961,6 +998,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..41a77f71671 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -2394,10 +2394,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/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index 72b8147f849..213b9fa96ab 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -513,13 +513,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; @@ -534,6 +538,7 @@ alter table parted_conflict_test attach partition parted_conflict_test_2 for val 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; -- should see (3, 'b') select * from parted_conflict_test order by a; @@ -544,6 +549,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; @@ -554,6 +560,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; @@ -566,6 +573,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..f79423ec86d 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -952,11 +952,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/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 432509277c9..5efaca672e1 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1813,9 +1813,9 @@ OldToNewMappingData OnCommitAction OnCommitItem OnConflictAction +OnConflictActionState OnConflictClause OnConflictExpr -OnConflictSetState OpClassCacheEnt OpExpr OpFamilyMember -- 2.51.0