Re: Added schema level support for publication. - Mailing list pgsql-hackers

On Tue, Aug 31, 2021 at 1:41 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, Aug 30, 2021 at 2:14 PM Greg Nancarrow <gregn4422@gmail.com> wrote:
> >
> > On Fri, Aug 27, 2021 at 4:13 PM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > I have implemented this in the 0003 patch, I have kept it separate to
> > > reduce the testing effort and also it will be easier if someone
> > > disagrees with the syntax. I will merge it to the main patch later
> > > based on the feedback. Attached v22 patch has the changes for the
> > > same.
> >
> > I notice that "CREATE PUBLICATION pub1 FOR ALL TABLES IN SCHEMA sc1,
> > TABLE sc1.test;"  maintains the table separately and results in the
> > following in the \dRp+ output:
> >
> > Tables:
> >     "sc1.test"
> > Schemas:
> >     "sc1"
> >
> > and also then "ALTER PUBLICATION pub1 DROP ALL TABLES IN SCHEMA sc1;"
> > still leaves the "sc1.test" table in the publication.
>
> I had intentionally implemented this way, the reason being it gives
> the flexibility to modify the publications based on the way the
> publication is created. My idea was that if a user specified a
> table/schema of the same schema while creating the publication, the
> user should be allowed to drop any of them at any time. In the above
> case if we don't maintain the results separately, users will not be
> able to drop the table from the publication at a later point of time.
> Thoughts?

I think ALL should mean ALL.

When you say CREATE PUBLICATION pub1 FOR ALL TABLES; you know it means
ALL tables;

When you say CREATE PUBLICATION pub1 FOR ALL TABLES IN SCHEMA sc1; you
know it means ALL tables in the schema sc1;

Similarly, when you say ALTER PUBLICATION pub1 DROP ALL TABLES IN
SCHEMA sc1; I would expect it means to drop ALL tables in sc1 - not
all of them sometimes but not all of them at other times or even none
of them.

~~

I thought a motivation for this patch was to make it easy for the user
to specify tables en-masse. e.g, saying FOR ALL TABLES IN SCHEMA sc1
is a convenience instead of having to specify every schema table
explicitly.

e.g. What if there are 100s of tables explicitly in a publication.
1. CREATE PUBLICATION pub FOR TABLE sc1.t1,sc1,t2,sc1.t3,....,sc1.t999;
2. ALTER PUBLICATION pub DROP ALL TABLES IN SCHEMA sc1;

IIUC the current patch behaviour for step 2 will do nothing at all.
That doesn't seem right to me. Where is the user-convenience factor
for dropping tables here?

~~

If the rule was simply "ALL means ALL" that hardly even needs
documentation to describe it. OTOH, current patch behaviour is quirky
wrt how the publication was created and would need to be carefully
documented.

It is easy to imagine a user unfamiliar with how the publication was
originally created will be confused when ALTER PUBLICATION DROP ALL
TABLES IN SCHEMA sc1 still leaves some sc1 tables lurking.

~~

Schema objects are not part of the publication. Current only TABLES
are in publications, so I thought that \dRp+ output would just be the
of "Tables" in the publication. Schemas would not even be displayed at
all (except in the table name).

Consider that everything is just going to get messier when SEQUENCES
are added. If you list Schemas like is done now then what's that going
to look like later? I think you'd need to display many lists like -
"Tables" and "Tables for Schemas" and "Sequences" and "Sequences for
Schema"...

IMO it's all beginning to sound like it would become overly complex

~~

For all the above reasons I think ALL should mean ALL, and also \dRp+
should display tables only. (and later table and sequences only)

YMMV.

------
Kind Regards,
Peter Smith
Fujitsu Australia