Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 26b3455

Browse files
author
Amit Kapila
committed
Fix partition table's REPLICA IDENTITY checking on the subscriber.
In logical replication, we will check if the target table on the subscriber is updatable by comparing the replica identity of the table on the publisher with the table on the subscriber. When the target table is a partitioned table, we only check its replica identity but not for the partition tables. This leads to assertion failure while applying changes for update/delete as we expect those to succeed only when the corresponding partition table has a primary key or has a replica identity defined. Fix it by checking the replica identity of the partition table while applying changes. Reported-by: Shi Yu Author: Shi Yu, Hou Zhijie Reviewed-by: Amit Langote, Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
1 parent 2253f5b commit 26b3455

File tree

3 files changed

+101
-55
lines changed

3 files changed

+101
-55
lines changed

src/backend/replication/logical/relation.c

+66-49
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,67 @@ logicalrep_report_missing_attrs(LogicalRepRelation *remoterel,
249249
}
250250
}
251251

252+
/*
253+
* Check if replica identity matches and mark the updatable flag.
254+
*
255+
* We allow for stricter replica identity (fewer columns) on subscriber as
256+
* that will not stop us from finding unique tuple. IE, if publisher has
257+
* identity (id,timestamp) and subscriber just (id) this will not be a
258+
* problem, but in the opposite scenario it will.
259+
*
260+
* We just mark the relation entry as not updatable here if the local
261+
* replica identity is found to be insufficient for applying
262+
* updates/deletes (inserts don't care!) and leave it to
263+
* check_relation_updatable() to throw the actual error if needed.
264+
*/
265+
static void
266+
logicalrep_rel_mark_updatable(LogicalRepRelMapEntry *entry)
267+
{
268+
Bitmapset *idkey;
269+
LogicalRepRelation *remoterel = &entry->remoterel;
270+
int i;
271+
272+
entry->updatable = true;
273+
274+
idkey = RelationGetIndexAttrBitmap(entry->localrel,
275+
INDEX_ATTR_BITMAP_IDENTITY_KEY);
276+
/* fallback to PK if no replica identity */
277+
if (idkey == NULL)
278+
{
279+
idkey = RelationGetIndexAttrBitmap(entry->localrel,
280+
INDEX_ATTR_BITMAP_PRIMARY_KEY);
281+
282+
/*
283+
* If no replica identity index and no PK, the published table must
284+
* have replica identity FULL.
285+
*/
286+
if (idkey == NULL && remoterel->replident != REPLICA_IDENTITY_FULL)
287+
entry->updatable = false;
288+
}
289+
290+
i = -1;
291+
while ((i = bms_next_member(idkey, i)) >= 0)
292+
{
293+
int attnum = i + FirstLowInvalidHeapAttributeNumber;
294+
295+
if (!AttrNumberIsForUserDefinedAttr(attnum))
296+
ereport(ERROR,
297+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
298+
errmsg("logical replication target relation \"%s.%s\" uses "
299+
"system columns in REPLICA IDENTITY index",
300+
remoterel->nspname, remoterel->relname)));
301+
302+
attnum = AttrNumberGetAttrOffset(attnum);
303+
304+
if (entry->attrmap->attnums[attnum] < 0 ||
305+
!bms_is_member(entry->attrmap->attnums[attnum], remoterel->attkeys))
306+
{
307+
entry->updatable = false;
308+
break;
309+
}
310+
}
311+
}
312+
252313
/*
253314
* Open the local relation associated with the remote one.
254315
*
@@ -307,7 +368,6 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
307368
if (!entry->localrelvalid)
308369
{
309370
Oid relid;
310-
Bitmapset *idkey;
311371
TupleDesc desc;
312372
MemoryContext oldctx;
313373
int i;
@@ -366,54 +426,10 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
366426
bms_free(missingatts);
367427

368428
/*
369-
* Check that replica identity matches. We allow for stricter replica
370-
* identity (fewer columns) on subscriber as that will not stop us
371-
* from finding unique tuple. IE, if publisher has identity
372-
* (id,timestamp) and subscriber just (id) this will not be a problem,
373-
* but in the opposite scenario it will.
374-
*
375-
* Don't throw any error here just mark the relation entry as not
376-
* updatable, as replica identity is only for updates and deletes but
377-
* inserts can be replicated even without it.
429+
* Set if the table's replica identity is enough to apply
430+
* update/delete.
378431
*/
379-
entry->updatable = true;
380-
idkey = RelationGetIndexAttrBitmap(entry->localrel,
381-
INDEX_ATTR_BITMAP_IDENTITY_KEY);
382-
/* fallback to PK if no replica identity */
383-
if (idkey == NULL)
384-
{
385-
idkey = RelationGetIndexAttrBitmap(entry->localrel,
386-
INDEX_ATTR_BITMAP_PRIMARY_KEY);
387-
388-
/*
389-
* If no replica identity index and no PK, the published table
390-
* must have replica identity FULL.
391-
*/
392-
if (idkey == NULL && remoterel->replident != REPLICA_IDENTITY_FULL)
393-
entry->updatable = false;
394-
}
395-
396-
i = -1;
397-
while ((i = bms_next_member(idkey, i)) >= 0)
398-
{
399-
int attnum = i + FirstLowInvalidHeapAttributeNumber;
400-
401-
if (!AttrNumberIsForUserDefinedAttr(attnum))
402-
ereport(ERROR,
403-
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
404-
errmsg("logical replication target relation \"%s.%s\" uses "
405-
"system columns in REPLICA IDENTITY index",
406-
remoterel->nspname, remoterel->relname)));
407-
408-
attnum = AttrNumberGetAttrOffset(attnum);
409-
410-
if (entry->attrmap->attnums[attnum] < 0 ||
411-
!bms_is_member(entry->attrmap->attnums[attnum], remoterel->attkeys))
412-
{
413-
entry->updatable = false;
414-
break;
415-
}
416-
}
432+
logicalrep_rel_mark_updatable(entry);
417433

418434
entry->localrelvalid = true;
419435
}
@@ -651,7 +667,8 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
651667
attrmap->maplen * sizeof(AttrNumber));
652668
}
653669

654-
entry->updatable = root->updatable;
670+
/* Set if the table's replica identity is enough to apply update/delete. */
671+
logicalrep_rel_mark_updatable(entry);
655672

656673
entry->localrelvalid = true;
657674

src/backend/replication/logical/worker.c

+21-6
Original file line numberDiff line numberDiff line change
@@ -1738,6 +1738,13 @@ apply_handle_insert_internal(ApplyExecutionData *edata,
17381738
static void
17391739
check_relation_updatable(LogicalRepRelMapEntry *rel)
17401740
{
1741+
/*
1742+
* For partitioned tables, we only need to care if the target partition is
1743+
* updatable (aka has PK or RI defined for it).
1744+
*/
1745+
if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
1746+
return;
1747+
17411748
/* Updatable, no error. */
17421749
if (rel->updatable)
17431750
return;
@@ -2121,6 +2128,8 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
21212128
TupleTableSlot *remoteslot_part;
21222129
TupleConversionMap *map;
21232130
MemoryContext oldctx;
2131+
LogicalRepRelMapEntry *part_entry = NULL;
2132+
AttrMap *attrmap = NULL;
21242133

21252134
/* ModifyTableState is needed for ExecFindPartition(). */
21262135
edata->mtstate = mtstate = makeNode(ModifyTableState);
@@ -2152,15 +2161,26 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
21522161
remoteslot_part = table_slot_create(partrel, &estate->es_tupleTable);
21532162
map = partrelinfo->ri_RootToPartitionMap;
21542163
if (map != NULL)
2155-
remoteslot_part = execute_attr_map_slot(map->attrMap, remoteslot,
2164+
{
2165+
attrmap = map->attrMap;
2166+
remoteslot_part = execute_attr_map_slot(attrmap, remoteslot,
21562167
remoteslot_part);
2168+
}
21572169
else
21582170
{
21592171
remoteslot_part = ExecCopySlot(remoteslot_part, remoteslot);
21602172
slot_getallattrs(remoteslot_part);
21612173
}
21622174
MemoryContextSwitchTo(oldctx);
21632175

2176+
/* Check if we can do the update or delete on the leaf partition. */
2177+
if (operation == CMD_UPDATE || operation == CMD_DELETE)
2178+
{
2179+
part_entry = logicalrep_partition_open(relmapentry, partrel,
2180+
attrmap);
2181+
check_relation_updatable(part_entry);
2182+
}
2183+
21642184
switch (operation)
21652185
{
21662186
case CMD_INSERT:
@@ -2182,15 +2202,10 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
21822202
* suitable partition.
21832203
*/
21842204
{
2185-
AttrMap *attrmap = map ? map->attrMap : NULL;
2186-
LogicalRepRelMapEntry *part_entry;
21872205
TupleTableSlot *localslot;
21882206
ResultRelInfo *partrelinfo_new;
21892207
bool found;
21902208

2191-
part_entry = logicalrep_partition_open(relmapentry, partrel,
2192-
attrmap);
2193-
21942209
/* Get the matching local tuple from the partition. */
21952210
found = FindReplTupleInLocalRel(estate, partrel,
21962211
&part_entry->remoterel,

src/test/subscription/t/013_partition.pl

+14
Original file line numberDiff line numberDiff line change
@@ -868,4 +868,18 @@ BEGIN
868868
"SELECT a, b, c FROM tab5 ORDER BY 1");
869869
is($result, qq(3||1), 'updates of tab5 replicated correctly after altering table on publisher');
870870

871+
# Test that replication works correctly as long as the leaf partition
872+
# has the necessary REPLICA IDENTITY, even though the actual target
873+
# partitioned table does not.
874+
$node_subscriber2->safe_psql('postgres',
875+
"ALTER TABLE tab5 REPLICA IDENTITY NOTHING");
876+
877+
$node_publisher->safe_psql('postgres', "UPDATE tab5 SET a = 4 WHERE a = 3");
878+
879+
$node_publisher->wait_for_catchup('sub2');
880+
881+
$result = $node_subscriber2->safe_psql('postgres',
882+
"SELECT a, b, c FROM tab5_1 ORDER BY 1");
883+
is($result, qq(4||1), 'updates of tab5 replicated correctly');
884+
871885
done_testing();

0 commit comments

Comments
 (0)