From ff466fcd130d8152c24182d449d1b896b3bc48d2 Mon Sep 17 00:00:00 2001 From: Hou Zhijie Date: Mon, 24 Mar 2025 16:40:26 +0800 Subject: [PATCH v1] Fix crashes in logical replication during unique constraint violations on leaf partitions. This commit addresses a logical replication crash introduced after commit 9ff6867, which occurs when incoming INSERT or UPDATE operations violate unique constraints on leaf partitions. The issue arises because the unique key conflict detection depended on the last ExecOpenIndices call to construct necessary index information, where the speculative parameter was set to true. However, this index opening was redundant because the indexes were already opened during target partition identification via the parent table (e.g., through ExecFindPartition) and hence was removed in 9ff6867. Unfortunately, ExecFindPartition opens indexes without constructing index information required for conflict detection, leading to the crash. To address this, a detect_conflict flag is introduced in ModifyTableState, enabling ExecFindPartition() to internally build the required index information for conflict detection. --- src/backend/executor/execPartition.c | 2 +- src/backend/replication/logical/worker.c | 2 ++ src/include/nodes/execnodes.h | 9 ++++++ src/test/subscription/t/035_conflicts.pl | 37 +++++++++++++++++++++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 84ccd7d457d..2eb8af0e0da 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -539,7 +539,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, if (partrel->rd_rel->relhasindex && leaf_part_rri->ri_IndexRelationDescs == NULL) ExecOpenIndices(leaf_part_rri, - (node != NULL && + mtstate->detect_conflict || (node != NULL && node->onConflictAction != ONCONFLICT_NONE)); /* diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index e3b2b144942..b2013929756 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -2990,6 +2990,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, mtstate->ps.state = estate; mtstate->operation = operation; mtstate->resultRelInfo = relinfo; + mtstate->detect_conflict = (operation == CMD_INSERT || + operation == CMD_UPDATE); /* ... as is PartitionTupleRouting. */ edata->proute = proute = ExecSetupPartitionTupleRouting(estate, parentrel); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index e42f9f9f957..0e33a90e676 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1458,6 +1458,15 @@ typedef struct ModifyTableState List *mt_updateColnosLists; List *mt_mergeActionLists; List *mt_mergeJoinConditions; + + /* + * Whether to detect conflicts within the target relation and potentially + * take actions other than throwing an error. + * + * Primarily used in scenarios without a real plan node, like the logical + * replication apply worker. + */ + bool detect_conflict; } ModifyTableState; /* ---------------- diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 3a4d44e1d0e..2a7a8239a29 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -25,14 +25,23 @@ $node_subscriber->start; $node_publisher->safe_psql('postgres', "CREATE TABLE conf_tab (a int PRIMARY KEY, b int UNIQUE, c int UNIQUE);"); +$node_publisher->safe_psql('postgres', + "CREATE TABLE conf_tab_2 (a int PRIMARY KEY, b int UNIQUE, c int UNIQUE);"); + # Create same table on subscriber $node_subscriber->safe_psql('postgres', "CREATE TABLE conf_tab (a int PRIMARY key, b int UNIQUE, c int UNIQUE);"); +$node_subscriber->safe_psql( + 'postgres', qq[ + CREATE TABLE conf_tab_2 (a int PRIMARY KEY, b int, c int, unique(a,b)) PARTITION BY RANGE (a); + CREATE TABLE conf_tab_2_p1 PARTITION OF conf_tab_2 FOR VALUES FROM (MINVALUE) TO (100); +]); + # Setup logical replication my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; $node_publisher->safe_psql('postgres', - "CREATE PUBLICATION pub_tab FOR TABLE conf_tab"); + "CREATE PUBLICATION pub_tab FOR TABLE conf_tab, conf_tab_2"); # Create the subscription my $appname = 'sub_tab'; @@ -110,4 +119,30 @@ $node_subscriber->wait_for_log( pass('multiple_unique_conflicts detected during update'); +# Truncate table to get rid of the error +$node_subscriber->safe_psql('postgres', "TRUNCATE conf_tab;"); + + +################################################## +# Test multiple_unique_conflicts due to INSERT on a leaf partition +################################################## + +# Insert data in the subscriber table +$node_subscriber->safe_psql('postgres', + "INSERT INTO conf_tab_2 VALUES (55,2,3);"); + +# Insert data in the publisher table +$node_publisher->safe_psql('postgres', + "INSERT INTO conf_tab_2 VALUES (55,2,3);"); + +$node_subscriber->wait_for_log( + qr/conflict detected on relation \"public.conf_tab_2_p1\": conflict=multiple_unique_conflicts.* +.*Key already exists in unique index \"conf_tab_2_p1_pkey\".* +.*Key \(a\)=\(55\); existing local tuple \(55, 2, 3\); remote tuple \(55, 2, 3\).* +.*Key already exists in unique index \"conf_tab_2_p1_a_b_key\".* +.*Key \(a, b\)=\(55, 2\); existing local tuple \(55, 2, 3\); remote tuple \(55, 2, 3\)./, + $log_offset); + +pass('multiple_unique_conflicts detected on a leaf partition during insert'); + done_testing(); -- 2.30.0.windows.2