From 361c362bfb5b08df729ade30533c25a351e34eee Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Wed, 20 Aug 2025 18:36:21 +0800 Subject: [PATCH v2 2/2] Fix replica identity check for INSERT ON CONFLICT This commit enforces proper checks for INSERT ON CONFLICT DO UPDATE command to determine if it can be executed with current replica identity. Previously, only the executability of the INSERT command was validated, missing checks for the UPDATE operations that can occur within this command. To fix it, validate the executability of the UPDATE operation as part of the ON CONFLICT DO UPDATE command. --- src/backend/commands/copyfrom.c | 2 +- src/backend/executor/execMain.c | 9 +++++++- src/backend/executor/execPartition.c | 9 ++++++-- src/backend/executor/nodeModifyTable.c | 3 ++- src/include/executor/executor.h | 3 ++- src/test/regress/expected/publication.out | 23 ++++++++++++++++++++ src/test/regress/sql/publication.sql | 26 +++++++++++++++++++++++ 7 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index fbbbc09a97b..9033d764d27 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -919,7 +919,7 @@ CopyFrom(CopyFromState cstate) ExecInitResultRelation(estate, resultRelInfo, 1); /* Verify the named relation is a valid target for INSERT */ - CheckValidResultRel(resultRelInfo, CMD_INSERT, NIL); + CheckValidResultRel(resultRelInfo, CMD_INSERT, NIL, ONCONFLICT_NONE); ExecOpenIndices(resultRelInfo, false); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index dee91ed40a9..ab3655bf3f0 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1045,7 +1045,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) */ void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, - List *mergeActions) + List *mergeActions, OnConflictAction onConflictAction) { Relation resultRel = resultRelInfo->ri_RelationDesc; FdwRoutine *fdwroutine; @@ -1069,6 +1069,13 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, CheckCmdReplicaIdentity(resultRel, action->commandType); else CheckCmdReplicaIdentity(resultRel, operation); + + /* + * For INSERT ON CONFLICT DO UPDATE, additionally verify whether + * the UPDATE can be executed for the target relation. + */ + if (onConflictAction == ONCONFLICT_UPDATE) + CheckCmdReplicaIdentity(resultRel, CMD_UPDATE); break; case RELKIND_SEQUENCE: ereport(ERROR, diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 514eae1037d..faed251aa89 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -360,8 +360,12 @@ ExecFindPartition(ModifyTableState *mtstate, true, false); if (rri) { + ModifyTable *node = (ModifyTable *) mtstate->ps.plan; + /* Verify this ResultRelInfo allows INSERTs */ - CheckValidResultRel(rri, CMD_INSERT, NIL); + CheckValidResultRel(rri, CMD_INSERT, NIL, node + ? node->onConflictAction + : ONCONFLICT_NONE); /* * Initialize information needed to insert this and @@ -527,7 +531,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); + CheckValidResultRel(leaf_part_rri, CMD_INSERT, NIL, + node ? node->onConflictAction : ONCONFLICT_NONE); /* * 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 7c6c2c1f6e4..bf5b9b8bca8 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -4811,7 +4811,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* * Verify result relation is a valid target for the current operation */ - CheckValidResultRel(resultRelInfo, operation, mergeActions); + CheckValidResultRel(resultRelInfo, operation, mergeActions, + node->onConflictAction); resultRelInfo++; i++; diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 10dcea037c3..81dbcf1b22f 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -244,7 +244,8 @@ extern bool ExecCheckPermissions(List *rangeTable, List *rteperminfos, bool ereport_on_violation); extern bool ExecCheckOneRelPerms(RTEPermissionInfo *perminfo); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, - List *mergeActions); + List *mergeActions, + OnConflictAction onConflictAction); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 702288f9a15..fe0bf8bcec7 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -1950,6 +1950,29 @@ MERGE INTO testpub_merge_pk USING testpub_merge_no_ri s ON s.a >= 1 DROP PUBLICATION pub1; DROP TABLE testpub_merge_no_ri; DROP TABLE testpub_merge_pk; +-- 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 edfef46e15e..ffa96b77257 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -1253,6 +1253,32 @@ DROP PUBLICATION pub1; DROP TABLE testpub_merge_no_ri; DROP TABLE testpub_merge_pk; +-- 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; -- 2.31.1