Re: pg_get_publication_tables() output duplicate relid

On Fri, Dec 3, 2021 at 6:04 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Fri, Dec 3, 2021 at 12:37 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Dec 2, 2021 at 7:18 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > > On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > On Thu, Dec 2, 2021 at 8:42 AM Amit Langote <amitlangote09@gmail.com> wrote:
> > > > > Reading Alvaro's email at the top again gave me a pause to reconsider
> > > > > what I had said in reply.  It might indeed have been nice if the
> > > > > publication DDL itself had prevented the cases where a partition and
> > > > > its ancestor are added to a publication, though as Hou-san mentioned,
> > > > > partition ATTACHes make that a bit tricky to enforce at all times,
> > > > > though of course not impossible.  Maybe it's okay to just de-duplicate
> > > > > pg_publication_tables output as the patch does, even though it may
> > > > > appear to be a band-aid solution if we one considers Alvaro's point
> > > > > about consistency.
> > > >
> > > > True, I think even if we consider that idea it will be a much bigger
> > > > change, and also as it will be a behavioral change so we might want to
> > > > keep it just for HEAD and some of these fixes need to be backpatched.
> > > > Having said that, if you or someone want to pursue that idea and come
> > > > up with a better solution than what we have currently it is certainly
> > > > welcome.
> > >
> > > Okay, I did write a PoC patch this morning after sending out my
> > > earlier email.  I polished it a bit, which is attached.
> >
> > I see multiple problems with this patch and idea.
>
> Thanks for looking at it.  Yeah, I have not looked very closely at ALL
> TABLES [IN SCHEMA], though only because I suspected that those cases
> deal with partitioning in such a way that the partition duplication
> issue doesn't arise.  That is, only the FOR TABLE list_of_tables and
> ADD TABLE syntax allow for the duplication issue to occur.
>
> > (a) I think you
> > forgot to deal with "All Tables In Schema" Publication which will be
> > quite tricky to deal with during attach operation. How will you remove
> > a particular relation from such a publication if there is a need to do
> > so?
>
> Hmm, my understanding of how FOR ALL TABLES... features work is that
> one cannot remove a particular relation from such publications?
>
> create schema sch;
> create table sch.p (a int primary key) partition by list (a);
> create table sch.p1 partition of sch.p for values in (1);
> create table sch.p2 partition of sch.p for values in (2);
> create table p (a int primary key) partition by list (a);
> create table p1 partition of p for values in (1);
> create table p2 partition of p for values in (2);
> create publication puball for all tables;
> create publication pubsch for all tables in schema sch;
>
> alter publication puball drop table p;
> ERROR:  publication "puball" is defined as FOR ALL TABLES
> DETAIL:  Tables cannot be added to or dropped from FOR ALL TABLES publications.
>
> alter publication pubsch drop table sch.p;
> ERROR:  relation "p" is not part of the publication
>
> What am I missing?
>

Currently, in your patch, you are trying to remove a particular
relation/partition during attach but how will you do that if such a
relation is part of All Tables In Schema publication?

> > (b) Also, how will we prohibit adding partition and its root in
> > the case of "All Tables In Schema" or "All Tables" publication? If
> > there is no way to do that, then that would mean we anyway need to
> > deal with parent and child tables as part of the same publication and
> > this new behavior will add an exception to it.
>
> I checked that FOR ALL TABLES publications don't allow adding a table
> explicitly, but apparently IN SCHEMA one does:
>
> alter publication pubsch add table p2;
> \dRp+ pubsch
>                            Publication pubsch
>  Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root
> -------+------------+---------+---------+---------+-----------+----------
>  amit  | f          | t       | t       | t       | t         | f
> Tables:
>     "public.p2"
> Tables from schemas:
>     "sch"
>
> ISTM that the ..IN SCHEMA syntax does not behave like FOR ALL TABLES
> without the IN SCHEMA in this regard.  Is that correct?
>

We do allow adding additional tables or schema to an exiting All
tables in schema publication.

-- 
With Regards,
Amit Kapila.