Re: Invalid Assert while validating REPLICA IDENTITY?

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Invalid Assert while validating REPLICA IDENTITY?
Date: 2024-09-09 06:14:42
Message-ID: CAFiTN-s_FPWCt3Z4jifDL=Aq-CvhEyzN5Gbv=a190ma1gY5W7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 6, 2024 at 4:48 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 2 Sept 2024 at 18:22, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Mon, Sep 2, 2024 at 3:32 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Sep 2, 2024 at 11:21 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > > While working on some other code I noticed that in
> > > > FindReplTupleInLocalRel() there is an assert [1] that seems to be
> > > > passing IndexRelation to GetRelationIdentityOrPK() whereas it should
> > > > be passing normal relation.
> > > >
> > >
> > > Agreed. But this should lead to assertion failure. Did you try testing it?
> >
> > No, I did not test this particular case, it impacted me with my other
> > addition of the code where I got Index Relation as input to the
> > RelationGetIndexList() function, and my local changes were impacted by
> > that. I will write a test for this stand-alone case so that it hits
> > the assert. Thanks for looking into this.
>
> The FindReplTupleInLocalRel function can be triggered by both update
> and delete operations, but this only occurs if the relation has been
> marked as updatable by the logicalrep_rel_mark_updatable function. If
> the relation is marked as non-updatable, an error will be thrown by
> check_relation_updatable. Given this, if a relation is updatable, the
> IsIndexUsableForReplicaIdentityFull condition might always evaluate to
> true due to the previous checks in logicalrep_rel_mark_updatable.
> Therefore, it’s possible that we might not encounter the Assert
> statement, as IsIndexUsableForReplicaIdentityFull may consistently be
> true.
> Thoughts?

With that it seems that the first Assert condition is useless isn't it?

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2024-09-09 06:16:39 Re: Allow logical failover slots to wait on synchronous replication
Previous Message Nisha Moond 2024-09-09 06:11:13 Re: Commit Timestamp and LSN Inversion issue