Horton

Re: Replica Identity check of partition table on subscriber

Amit Langote <amitlangote09@gmail.com>

View thread View on postgr.es
From: Amit Langote <amitlangote09@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Date: 2022-06-16T07:00:23Z
Lists: pgsql-hackers 
On Thu, Jun 16, 2022 at 3:45 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Jun 16, 2022 at 11:43 AM Amit Langote <amitlangote09@gmail.com> wrote:
> > + * Don't throw any error here just mark the relation entry as not updatable,
> > + * as replica identity is only for updates and deletes but inserts can be
> > + * replicated even without it.
> >
> > I know you're simply copying the old comment, but I think we can
> > rewrite it to be slightly more useful:
> >
> > We just mark the relation entry as not updatable here if the local
> > replica identity is found to be insufficient and leave it to
> > check_relation_updatable() to throw the actual error if needed.
>
> I am fine with improving this comment but it would be better if in
> some way we keep the following part of the comment: "as replica
> identity is only for updates and deletes but inserts can be replicated
> even without it." as that makes it more clear why it is okay to just
> mark the entry as not updatable. One idea could be: "We just mark the
> relation entry as not updatable here if the local replica identity is
> found to be insufficient and leave it to check_relation_updatable() to
> throw the actual error if needed. This is because replica identity is
> only for updates and deletes but inserts can be replicated even
> without it.". Feel free to suggest if you have any better ideas?

I thought mentioning check_relation_updatable() would make it clear
that only updates (and deletes) care about a valid local replica
identity, because only apply_handle_{update|delete}() call that
function.  Anyway, how about this:

We just mark the relation entry as not updatable here if the local
replica identity is found to be insufficient for applying
updates/deletes (inserts don't care!) and leave it to
check_relation_updatable() to throw the actual error if needed.

-- 
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Commits

  1. Fix memory leak due to LogicalRepRelMapEntry.attrmap.

  2. Fix stale values in partition map entries on subscribers.

  3. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  4. Fix data inconsistency between publisher and subscriber.

  5. Fix cache look-up failures while applying changes in logical replication.

  6. Add logical replication support to replicate into partitioned tables