On Fri, Dec 3, 2021 at 6:04 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Fri, Dec 3, 2021 at 12:37 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Dec 2, 2021 at 7:18 PM Amit Langote <amitlangot...@gmail.com> wrote: > > > On Thu, Dec 2, 2021 at 2:27 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > On Thu, Dec 2, 2021 at 8:42 AM Amit Langote <amitlangot...@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.
