Skip to content

Commit 12eece5

Browse files
author
Amit Kapila
committed
Fix uninitialized index information access during apply.
The issue happens when building conflict information during apply of INSERT or UPDATE operations that violate unique constraints on leaf partitions. The problem was introduced in commit 9ff6867, which removed the redundant calls to ExecOpenIndices/ExecCloseIndices. The previous code was relying on the redundant ExecOpenIndices call in apply_handle_tuple_routing() to build the index information required for unique key conflict detection. The fix is to delay building the index information until a conflict is detected instead of relying on ExecOpenIndices to do the same. The additional benefit of this approach is that it avoids building index information when there is no conflict. Author: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by:Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/TYAPR01MB57244ADA33DDA57119B9D26494A62@TYAPR01MB5724.jpnprd01.prod.outlook.com
1 parent 7ea21f4 commit 12eece5

File tree

4 files changed

+70
-6
lines changed

4 files changed

+70
-6
lines changed

‎src/backend/executor/execIndexing.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,8 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative)
214214
ii = BuildIndexInfo(indexDesc);
215215

216216
/*
217-
* If the indexes are to be used for speculative insertion or conflict
218-
* detection in logical replication, add extra information required by
219-
* unique index entries.
217+
* If the indexes are to be used for speculative insertion, add extra
218+
* information required by unique index entries.
220219
*/
221220
if (speculative && ii->ii_Unique && !indexDesc->rd_index->indisexclusion)
222221
BuildSpeculativeIndexInfo(indexDesc, ii);

‎src/backend/executor/execReplication.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,30 @@ RelationFindReplTupleSeq(Relation rel, LockTupleMode lockmode,
431431
return found;
432432
}
433433

434+
/*
435+
* Build additional index information necessary for conflict detection.
436+
*/
437+
static void
438+
BuildConflictIndexInfo(ResultRelInfo *resultRelInfo, Oid conflictindex)
439+
{
440+
for (int i = 0; i < resultRelInfo->ri_NumIndices; i++)
441+
{
442+
Relation indexRelation = resultRelInfo->ri_IndexRelationDescs[i];
443+
IndexInfo *indexRelationInfo = resultRelInfo->ri_IndexRelationInfo[i];
444+
445+
if (conflictindex != RelationGetRelid(indexRelation))
446+
continue;
447+
448+
/*
449+
* This Assert will fail if BuildSpeculativeIndexInfo() is called
450+
* twice for the given index.
451+
*/
452+
Assert(indexRelationInfo->ii_UniqueOps == NULL);
453+
454+
BuildSpeculativeIndexInfo(indexRelation, indexRelationInfo);
455+
}
456+
}
457+
434458
/*
435459
* Find the tuple that violates the passed unique index (conflictindex).
436460
*
@@ -452,6 +476,12 @@ FindConflictTuple(ResultRelInfo *resultRelInfo, EState *estate,
452476

453477
*conflictslot = NULL;
454478

479+
/*
480+
* Build additional information required to check constraints violations.
481+
* See check_exclusion_or_unique_constraint().
482+
*/
483+
BuildConflictIndexInfo(resultRelInfo, conflictindex);
484+
455485
retry:
456486
if (ExecCheckIndexConstraints(resultRelInfo, slot, estate,
457487
&conflictTid, &slot->tts_tid,

‎src/backend/replication/logical/worker.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2457,7 +2457,7 @@ apply_handle_insert(StringInfo s)
24572457
{
24582458
ResultRelInfo *relinfo = edata->targetRelInfo;
24592459

2460-
ExecOpenIndices(relinfo, true);
2460+
ExecOpenIndices(relinfo, false);
24612461
apply_handle_insert_internal(edata, relinfo, remoteslot);
24622462
ExecCloseIndices(relinfo);
24632463
}
@@ -2680,7 +2680,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
26802680
MemoryContext oldctx;
26812681

26822682
EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
2683-
ExecOpenIndices(relinfo, true);
2683+
ExecOpenIndices(relinfo, false);
26842684

26852685
found = FindReplTupleInLocalRel(edata, localrel,
26862686
&relmapentry->remoterel,

‎src/test/subscription/t/035_conflicts.pl

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,23 @@
2525
$node_publisher->safe_psql('postgres',
2626
"CREATE TABLE conf_tab (a int PRIMARY KEY, b int UNIQUE, c int UNIQUE);");
2727

28+
$node_publisher->safe_psql('postgres',
29+
"CREATE TABLE conf_tab_2 (a int PRIMARY KEY, b int UNIQUE, c int UNIQUE);");
30+
2831
# Create same table on subscriber
2932
$node_subscriber->safe_psql('postgres',
3033
"CREATE TABLE conf_tab (a int PRIMARY key, b int UNIQUE, c int UNIQUE);");
3134

35+
$node_subscriber->safe_psql(
36+
'postgres', qq[
37+
CREATE TABLE conf_tab_2 (a int PRIMARY KEY, b int, c int, unique(a,b)) PARTITION BY RANGE (a);
38+
CREATE TABLE conf_tab_2_p1 PARTITION OF conf_tab_2 FOR VALUES FROM (MINVALUE) TO (100);
39+
]);
40+
3241
# Setup logical replication
3342
my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
3443
$node_publisher->safe_psql('postgres',
35-
"CREATE PUBLICATION pub_tab FOR TABLE conf_tab");
44+
"CREATE PUBLICATION pub_tab FOR TABLE conf_tab, conf_tab_2");
3645

3746
# Create the subscription
3847
my $appname = 'sub_tab';
@@ -110,4 +119,30 @@
110119

111120
pass('multiple_unique_conflicts detected during update');
112121

122+
# Truncate table to get rid of the error
123+
$node_subscriber->safe_psql('postgres', "TRUNCATE conf_tab;");
124+
125+
126+
##################################################
127+
# Test multiple_unique_conflicts due to INSERT on a leaf partition
128+
##################################################
129+
130+
# Insert data in the subscriber table
131+
$node_subscriber->safe_psql('postgres',
132+
"INSERT INTO conf_tab_2 VALUES (55,2,3);");
133+
134+
# Insert data in the publisher table
135+
$node_publisher->safe_psql('postgres',
136+
"INSERT INTO conf_tab_2 VALUES (55,2,3);");
137+
138+
$node_subscriber->wait_for_log(
139+
qr/conflict detected on relation \"public.conf_tab_2_p1\": conflict=multiple_unique_conflicts.*
140+
.*Key already exists in unique index \"conf_tab_2_p1_pkey\".*
141+
.*Key \(a\)=\(55\); existing local tuple \(55, 2, 3\); remote tuple \(55, 2, 3\).*
142+
.*Key already exists in unique index \"conf_tab_2_p1_a_b_key\".*
143+
.*Key \(a, b\)=\(55, 2\); existing local tuple \(55, 2, 3\); remote tuple \(55, 2, 3\)./,
144+
$log_offset);
145+
146+
pass('multiple_unique_conflicts detected on a leaf partition during insert');
147+
113148
done_testing();

0 commit comments

Comments
 (0)