Re: pg_get_publication_tables() output duplicate relid

On Tue, Nov 16, 2021 at 10:27 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Mon, Nov 15, 2021 at 7:12 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > > On Mon, Nov 15, 2021 at 1:48 PM houzj.fnst@fujitsu.com
> > > <houzj.fnst@fujitsu.com> wrote:
> >
> > > > create table tbl1 (a int) partition by range (a);
> > > > create table tbl1_part1 partition of tbl1 for values from (1) to (10);
> > > > create table tbl1_part2 partition of tbl1 for values from (10) to (20);
> > > > create publication pub for table
> > > > tbl1, tbl1_part1 with (publish_via_partition_root=false);
> >
> > In the name of consistency, I think this situation should be an error --
> > I mean, if we detect that the user is trying to add both the partitioned
> > table *and* its partition, then all manner of things are possibly going
> > to go wrong in some way, so my inclination is to avoid it altogether.
> >
> > Is there any reason to allow that?
>
> I think it could provide flexibility to users to later change
> "publish_via_partition_root" option. Because when that option is
> false, we use individual partitions schema to send changes and when it
> is true, we use root table's schema to send changes. Added Amit L. to
> know if he has any thoughts on this matter as he was the author of
> this work?

FWIW, I'm not sure that adding an error in this path, that is, when a
user adds both a partitioned table and its partitions to a
publication, helps much.  As for the safety of allowing it, if you
look at get_rel_sync_entry(), which handles much of the complexity of
determining whether to publish a relation's changes and the schema to
use when doing so, you may be able to see that a partition being added
duplicatively is harmless, modulo any undiscovered bugs.  At least as
far as the post-initial-sync replication functionality is concerned.

What IS problematic is what a subscriber sees in the
pg_publication_tables view and the problem occurs only in the initial
sync phase, where the partition is synced duplicatively because of
being found in the view along with the parent in this case, that is,
when publish_via_partiiton_root is true.  I was saying on the other
thread [1] that we should leave it up to the subscriber to decide what
to do when the view (duplicatively) returns both the parent and the
partition, citing the use case that a subscriber may want to replicate
the parent and the partition as independent tables.  Though I now tend
to agree with Amit K that that may be such a meaningful and all that
common use case, and the implementation on the subscriber side would
have to be unnecessarily complex.

So maybe it makes sense to just do what has been proposed --
de-duplicate partitions out of the pg_publication_tables view, unless
we know of a bigger problem that requires us to hack the subscriber
side of things too.  Actually, I came to know of one such problem
while thinking about this today: when you ATTACH a partition to a
table that is present in a publish_via_partition_root=true
publication, it doesn't get copied via the initial sync even though
subsequent replication works just fine.  The reason for that is that
the subscriber only syncs the partitions that are known at the time
when the parent is first synced, that too via the parent (as SELECT
<columns..> FROM parent), and then marks the parent as sync-done.
Refreshing the subscription after ATTACHing doesn't help, because the
subscriber can't see any partitions to begin with.  Maybe a more
elaborate solution is needed for this one, though I haven't figured
what it is going to look like yet.

Thanks.

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

[1] https://www.postgresql.org/message-id/CA%2BHiwqHnDHcT4OOcga9rDFyc7TvDrpN5xFH9J2pyHQo9ptvjmQ%40mail.gmail.com