Re: Added schema level support for publication.

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Subject: Re: Added schema level support for publication.
Date: 2021-09-02 05:33:47
Message-ID: CAA4eK1Jgv56WOmHHnOFVf36iJOfUsFLj-yJTFqzuz1pO3e9REw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 1, 2021 at 12:06 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> 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?
>

It will give an error (ERROR: schema "sc1" is not part of the
publication), we can probably add DETAIL and HINT message (like try
dropping using DROP TABLE syntax) indicating the reason and what user
need to do in this regard. I think if the user has specified tables
individually instead of ALL TABLES IN SCHEMA, she should drop them
individually rather than expecting them to be dropped via SCHEMA
command. I think in a similar context you can also argue that we
should have DROP ALL TABLES syntax to drop all the tables that were
individually specified by the user with FOR TABLE command.

I think this should be documented as well to avoid any confusion.

> ~~
>
> 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).
>

Hmm, I think this will lead to a lot of table names in output. I think
displaying schema names separately is a better approach here.

> 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"...
>

I think that would be better than displaying all the tables and
sequences as that will be very difficult for users to view/read.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-09-02 05:36:28 Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)
Previous Message Fujii Masao 2021-09-02 05:04:56 Re: Support reset of Shared objects statistics in "pg_stat_reset" function