diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index e35ddd0e898..05f8c70d878 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1008,6 +1008,9 @@ InitPlan(QueryDesc *queryDesc, int eflags) * Generally the parser and/or planner should have noticed any such mistake * already, but let's make sure. * + * For INSERT ON CONFLICT, the result relation is required to support the + * onConflictAction, regardless of whether a conflict actually occurs. + * * For MERGE, mergeActions is the list of actions that may be performed. The * result relation is required to support every action, regardless of whether * or not they are all executed. @@ -1016,8 +1019,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) * CheckValidRowMarkRel. */ void -CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, - List *mergeActions) +CheckValidResultRelNew(ResultRelInfo *resultRelInfo, CmdType operation, + OnConflictAction onConflictAction, List *mergeActions) { Relation resultRel = resultRelInfo->ri_RelationDesc; FdwRoutine *fdwroutine; @@ -1031,6 +1034,13 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, case RELKIND_RELATION: case RELKIND_PARTITIONED_TABLE: CheckCmdReplicaIdentity(resultRel, operation); + + /* + * For INSERT ON CONFLICT DO UPDATE, additionally check that the + * target relation supports UPDATE. + */ + if (onConflictAction == ONCONFLICT_UPDATE) + CheckCmdReplicaIdentity(resultRel, CMD_UPDATE); break; case RELKIND_SEQUENCE: ereport(ERROR, @@ -1121,6 +1131,18 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, } } +/* + * ABI-compatible wrapper to emulate old version of the above function. + * Do not call this version in new code. + */ +void +CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, + List *mergeActions) +{ + return CheckValidResultRelNew(resultRelInfo, operation, ONCONFLICT_NONE, + mergeActions); +} + /* * Check that a proposed rowmark target relation is a legal target * diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index a3602b81687..6c7e8839f98 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -357,8 +357,12 @@ ExecFindPartition(ModifyTableState *mtstate, true, false); if (rri) { + ModifyTable *node = (ModifyTable *) mtstate->ps.plan; + /* Verify this ResultRelInfo allows INSERTs */ - CheckValidResultRel(rri, CMD_INSERT, NIL); + CheckValidResultRelNew(rri, CMD_INSERT, + node ? node->onConflictAction : ONCONFLICT_NONE, + NIL); /* * Initialize information needed to insert this and @@ -524,7 +528,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * partition-key becomes a DELETE+INSERT operation, so this check is still * required when the operation is CMD_UPDATE. */ - CheckValidResultRel(leaf_part_rri, CMD_INSERT, NIL); + CheckValidResultRelNew(leaf_part_rri, CMD_INSERT, + node ? node->onConflictAction : ONCONFLICT_NONE, NIL); /* * Open partition indices. The user may have asked to check for conflicts diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c230b666706..80e99284657 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -4531,7 +4531,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* * Verify result relation is a valid target for the current operation */ - CheckValidResultRel(resultRelInfo, operation, mergeActions); + CheckValidResultRelNew(resultRelInfo, operation, + node->onConflictAction, mergeActions); resultRelInfo++; i++; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 7e6e366ceac..6cecfbcbdf9 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -211,6 +211,9 @@ extern void ExecutorRewind(QueryDesc *queryDesc); extern bool ExecCheckPermissions(List *rangeTable, List *rteperminfos, bool ereport_on_violation); extern bool ExecCheckOneRelPerms(RTEPermissionInfo *perminfo); +extern void CheckValidResultRelNew(ResultRelInfo *resultRelInfo, CmdType operation, + OnConflictAction onConflictAction, + List *mergeActions); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, List *mergeActions); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 3edf0bed510..10c3b393ebd 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -1742,6 +1742,29 @@ DROP PUBLICATION pub; DROP TABLE sch1.tbl1; DROP SCHEMA sch1 cascade; DROP SCHEMA sch2 cascade; +-- Test that the INSERT ON CONFLICT command correctly checks REPLICA IDENTITY +-- when the target table is published. +CREATE TABLE testpub_insert_onconfl_no_ri (a int unique, b int); +CREATE TABLE testpub_insert_onconfl_parted (a int unique, b int) PARTITION by RANGE (a); +CREATE TABLE testpub_insert_onconfl_part_no_ri PARTITION OF testpub_insert_onconfl_parted FOR VALUES FROM (1) TO (10); +SET client_min_messages = 'ERROR'; +CREATE PUBLICATION pub1 FOR ALL TABLES; +RESET client_min_messages; +-- fail - missing REPLICA IDENTITY +INSERT INTO testpub_insert_onconfl_no_ri VALUES (1, 1) ON CONFLICT (a) DO UPDATE SET b = 2; +ERROR: cannot update table "testpub_insert_onconfl_no_ri" because it does not have a replica identity and publishes updates +HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE. +-- ok - no updates +INSERT INTO testpub_insert_onconfl_no_ri VALUES (1, 1) ON CONFLICT DO NOTHING; +-- fail - missing REPLICA IDENTITY in partition testpub_insert_onconfl_no_ri +INSERT INTO testpub_insert_onconfl_parted VALUES (1, 1) ON CONFLICT (a) DO UPDATE SET b = 2; +ERROR: cannot update table "testpub_insert_onconfl_part_no_ri" because it does not have a replica identity and publishes updates +HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE. +-- ok - no updates +INSERT INTO testpub_insert_onconfl_parted VALUES (1, 1) ON CONFLICT DO NOTHING; +DROP PUBLICATION pub1; +DROP TABLE testpub_insert_onconfl_no_ri; +DROP TABLE testpub_insert_onconfl_parted; RESET SESSION AUTHORIZATION; DROP ROLE regress_publication_user, regress_publication_user2; DROP ROLE regress_publication_user_dummy; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index c4f12d4e0a3..689d3fddd53 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -1106,6 +1106,32 @@ DROP TABLE sch1.tbl1; DROP SCHEMA sch1 cascade; DROP SCHEMA sch2 cascade; +-- Test that the INSERT ON CONFLICT command correctly checks REPLICA IDENTITY +-- when the target table is published. +CREATE TABLE testpub_insert_onconfl_no_ri (a int unique, b int); +CREATE TABLE testpub_insert_onconfl_parted (a int unique, b int) PARTITION by RANGE (a); +CREATE TABLE testpub_insert_onconfl_part_no_ri PARTITION OF testpub_insert_onconfl_parted FOR VALUES FROM (1) TO (10); + +SET client_min_messages = 'ERROR'; +CREATE PUBLICATION pub1 FOR ALL TABLES; +RESET client_min_messages; + +-- fail - missing REPLICA IDENTITY +INSERT INTO testpub_insert_onconfl_no_ri VALUES (1, 1) ON CONFLICT (a) DO UPDATE SET b = 2; + +-- ok - no updates +INSERT INTO testpub_insert_onconfl_no_ri VALUES (1, 1) ON CONFLICT DO NOTHING; + +-- fail - missing REPLICA IDENTITY in partition testpub_insert_onconfl_no_ri +INSERT INTO testpub_insert_onconfl_parted VALUES (1, 1) ON CONFLICT (a) DO UPDATE SET b = 2; + +-- ok - no updates +INSERT INTO testpub_insert_onconfl_parted VALUES (1, 1) ON CONFLICT DO NOTHING; + +DROP PUBLICATION pub1; +DROP TABLE testpub_insert_onconfl_no_ri; +DROP TABLE testpub_insert_onconfl_parted; + RESET SESSION AUTHORIZATION; DROP ROLE regress_publication_user, regress_publication_user2; DROP ROLE regress_publication_user_dummy;