On Wed, Nov 24, 2021 at 6:51 AM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > > On Tues, Nov 23, 2021 6:16 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Nov 23, 2021 at 1:29 PM houzj.f...@fujitsu.com > > <houzj.f...@fujitsu.com> wrote: > > > > > > On Tues, Nov 23, 2021 2:27 PM vignesh C <vignes...@gmail.com> wrote: > > > > On Thu, Nov 18, 2021 at 7:04 AM Peter Smith <smithpb2...@gmail.com> > > > > wrote: > > > > > > > > > > PSA new set of v40* patches. > > > > > > > > Few comments: > > > > 1) When a table is added to the publication, replica identity is > > > > checked. But > > > > while modifying the publish action to include delete/update, replica > > > > identity is > > > > not checked for the existing tables. I felt it should be checked for > > > > the existing > > > > tables too. > > > > > > In addition to this, I think we might also need some check to prevent > > > user from > > > changing the REPLICA IDENTITY index which is used in the filter > > > expression. > > > > > > I was thinking is it possible do the check related to REPLICA IDENTITY in > > > function CheckCmdReplicaIdentity() or In GetRelationPublicationActions(). > > > If we > > > move the REPLICA IDENTITY check to this function, it would be consistent > > > with > > > the existing behavior about the check related to REPLICA IDENTITY(see the > > > comments in CheckCmdReplicaIdentity) and seems can cover all the cases > > > mentioned above. > > > > > > > Yeah, adding the replica identity check in CheckCmdReplicaIdentity() > > would cover all the above cases but I think that would put a premium > > on each update/delete operation. I think traversing the expression > > tree (it could be multiple traversals if the relation is part of > > multiple publications) during each update/delete would be costly. > > Don't you think so? > > Yes, I agreed that traversing the expression every time would be costly. > > I thought maybe we can cache the columns used in row filter or cache only the > a > flag(can_update|delete) in the relcache. I think every operation that affect > the row-filter or replica-identity will invalidate the relcache and the cost > of > check seems acceptable with the cache. >
I think if we can cache this information especially as a bool flag then that should probably be better. -- With Regards, Amit Kapila.
