Re: row filtering for logical replication

On Wed, Nov 24, 2021 at 6:51 AM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tues, Nov 23, 2021 6:16 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Tue, Nov 23, 2021 at 1:29 PM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Tues, Nov 23, 2021 2:27 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > > On Thu, Nov 18, 2021 at 7:04 AM Peter Smith <smithpb2250(at)gmail(dot)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.