On Mon, Mar 6, 2023 at 1:40 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Mon, Mar 6, 2023 at 5:44 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Mar 6, 2023 at 10:12 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > 4. IdxIsRelationIdentityOrPK > > > > > > +/* > > > + * Given a relation and OID of an index, returns true if the > > > + * index is relation's replica identity index or relation's > > > + * primary key's index. > > > + * > > > + * Returns false otherwise. > > > + */ > > > +bool > > > +IdxIsRelationIdentityOrPK(Relation rel, Oid idxoid) > > > +{ > > > + Assert(OidIsValid(idxoid)); > > > + > > > + return GetRelationIdentityOrPK(rel) == idxoid; > > > +} > > > > > > I think you've "simplified" this function in v28 but AFAICT now it has > > > a different logic to v27. > > > > > > PREVIOUSLY it was coded like > > > + return RelationGetReplicaIndex(rel) == idxoid || > > > + RelationGetPrimaryKeyIndex(rel) == idxoid; > > > > > > You can see if 'idxoid' did NOT match RI but if it DID match PK > > > previously it would return true. But now in that scenario, it won't > > > even check the PK if there was a valid RI. So it might return false > > > when previously it returned true. Is it deliberate? > > > > > > > I don't see any problem with this because by default PK will be a > > replica identity. So only if the user specifies the replica identity > > full or changes the replica identity to some other index, we will try > > to get PK which seems valid for this case. Am, I missing something > > which makes this code do something bad? > > I don't know if there is anything bad; the point was that the function > now seems to require a deeper understanding of the interrelationship > of RelationGetReplicaIndex and RelationGetPrimaryKeyIndex, which is > something the previous implementation did not require. >
But the same understanding is required for the existing function GetRelationIdentityOrPK(), so I feel it is better to be consistent unless we see some problem here. > > Anyhow, if you feel those firstterm and FULL changes ought to be kept > separate from this RI patch, please let me know and I will propose > those changes in a new thread, > Personally, I would prefer to keep those separate. So, feel free to propose them in a new thread. -- With Regards, Amit Kapila.