On Wed, May 13, 2020 at 6:15 AM Euler Taveira <euler.tave...@2ndquadrant.com> wrote: > > 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.
I like your idea of modifying the assert instead of completely removing. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
