On Tue, 12 May 2020 at 06:36, Masahiko Sawada < masahiko.saw...@2ndquadrant.com> wrote:
> On Mon, 11 May 2020 at 16:28, Michael Paquier <mich...@paquier.xyz> wrote: > > > > On Sun, May 10, 2020 at 07:08:03PM -0300, Euler Taveira wrote: > > > I attached a patch with the described solution. I also included a test > that > > > covers this scenario. > > > > - Assert(RelationGetReplicaIndex(rel) == RelationGetRelid(idxrel)); > > + Assert(GetRelationIdentityOrPK(rel) == RelationGetRelid(idxrel)); > > > > Not much a fan of adding a routine to relcache.c to do the work of two > > routines already present, so I think that we had better add an extra > > condition based on RelationGetPrimaryKeyIndex, and give up on > > GetRelationIdentityOrPK() in execReplication.c. > > Although, I think this solution is fragile, I updated the patch accordingly. (When/If someone changed GetRelationIdentityOrPK() it will break this assert) > In any case, it seems to me that the comment of > build_replindex_scan_key needs to be updated. > > * This is not generic routine, it expects the idxrel to be replication > * identity of a rel and meet all limitations associated with that. > > It is implicit that a primary key can be a replica identity so I think this comment is fine. -- Euler Taveira http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From a858f9253b4bb4ceb8a8c5cd93335499e3ad4673 Mon Sep 17 00:00:00 2001 From: Euler Taveira <euler.tave...@2ndquadrant.com> Date: Tue, 12 May 2020 19:40:48 -0300 Subject: [PATCH] Fix assert failure with REPLICA IDENTITY FULL in the subscriber This assertion failure can be reproduced using a replicated table in the subscriber with REPLICA IDENTITY FULL. Since FindReplTupleInLocalRel uses GetRelationIdentityOrPK to obtain an index and a few calls later this index is used in build_replindex_scan_key, this function cannot assert using a function other than GetRelationIdentityOrPK. Although, GetRelationIdentityOrPK is a generic function that uses other 2 functions; we can the same logic from GetRelationIdentityOrPK into Assert (thankfully the logic is simple). --- src/backend/executor/execReplication.c | 3 ++- src/test/subscription/t/001_rep_changes.pl | 12 +++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 1418746eb8..af8f0afb6f 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -57,7 +57,8 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel, int2vector *indkey = &idxrel->rd_index->indkey; bool hasnulls = false; - Assert(RelationGetReplicaIndex(rel) == RelationGetRelid(idxrel)); + Assert((RelationGetReplicaIndex(rel) == RelationGetRelid(idxrel)) || + (RelationGetPrimaryKeyIndex(rel) == RelationGetRelid(idxrel))); indclassDatum = SysCacheGetAttr(INDEXRELID, idxrel->rd_indextuple, Anum_pg_index_indclass, &isnull); diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl index 6da7f71ca3..7d9ea1f002 100644 --- a/src/test/subscription/t/001_rep_changes.pl +++ b/src/test/subscription/t/001_rep_changes.pl @@ -34,6 +34,8 @@ $node_publisher->safe_psql('postgres', $node_publisher->safe_psql('postgres', "CREATE TABLE tab_include (a int, b text, CONSTRAINT covering PRIMARY KEY(a) INCLUDE(b))" ); +$node_publisher->safe_psql('postgres', + "CREATE TABLE tab_full_pk (a int primary key, b text)"); # Let this table with REPLICA IDENTITY NOTHING, allowing only INSERT changes. $node_publisher->safe_psql('postgres', "CREATE TABLE tab_nothing (a int)"); $node_publisher->safe_psql('postgres', @@ -46,6 +48,9 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_full (a int)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_full2 (x text)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_rep (a int primary key)"); +$node_subscriber->safe_psql('postgres', + "CREATE TABLE tab_full_pk (a int primary key, b text)"); +$node_subscriber->safe_psql('postgres', "ALTER TABLE tab_full_pk REPLICA IDENTITY FULL"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_nothing (a int)"); # different column count and order than on publisher @@ -64,7 +69,7 @@ $node_publisher->safe_psql('postgres', "CREATE PUBLICATION tap_pub"); $node_publisher->safe_psql('postgres', "CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)"); $node_publisher->safe_psql('postgres', - "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_full2, tab_mixed, tab_include, tab_nothing" + "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_full2, tab_mixed, tab_include, tab_nothing, tab_full_pk" ); $node_publisher->safe_psql('postgres', "ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_ins"); @@ -102,6 +107,9 @@ $node_publisher->safe_psql('postgres', "UPDATE tab_rep SET a = -a"); $node_publisher->safe_psql('postgres', "INSERT INTO tab_mixed VALUES (2, 'bar', 2.2)"); +$node_publisher->safe_psql('postgres', + "INSERT INTO tab_full_pk VALUES (1, 'foo')"); + $node_publisher->safe_psql('postgres', "INSERT INTO tab_nothing VALUES (generate_series(1,20))"); @@ -159,6 +167,8 @@ $node_publisher->safe_psql('postgres', "UPDATE tab_full2 SET x = 'bb' WHERE x = 'b'"); $node_publisher->safe_psql('postgres', "UPDATE tab_mixed SET b = 'baz' WHERE a = 1"); +$node_publisher->safe_psql('postgres', + "UPDATE tab_full_pk SET b = 'bar' WHERE a = 1"); $node_publisher->wait_for_catchup('tap_sub'); -- 2.20.1