On Tue, Sep 20, 2022 at 2:57 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Sep-20, Amit Kapila wrote:
>
> > On Mon, Sep 19, 2022 at 8:46 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> > > This seems a pretty arbitrary restriction. It feels like you're adding
> > > this restriction precisely so that you don't have to write the code to
> > > reject the ALTER .. SET SCHEMA if an incompatible configuration is
> > > detected. But we already have such checks in several cases, so I don't
> > > see why this one does not seem a good idea.
> > >
> > I agree that we have such checks at other places as well and one
> > somewhat similar is in ATPrepChangePersistence().
> >
> > ATPrepChangePersistence()
> > {
> > ...
> > ...
> > /*
> > * Check that the table is not part of any publication when changing to
> > * UNLOGGED, as UNLOGGED tables can't be published.
> > */
>
> Right, I think this is a sensible approach.
>
> > However, another angle to look at it is that we try to avoid adding
> > restrictions in other DDL commands for defined publications.
>
> Well, it makes sense to avoid restrictions wherever possible. But here,
> the consequence is that you end up with a restriction in the publication
> definition that is not very sensible. Imagine if you said "you can't
> add schema S because it contains an unlogged table". It's absurd.
>
> Maybe this can be relaxed in a future release, but it's quite odd.
>
Yeah, we can relax it in a future release based on some field
experience, or maybe we can keep the current restriction of not
allowing to add a table when the schema of the table is part of the
same publication and try to relax that in a future release based on
field experience.
> > The intention was to be in sync with FOR ALL TABLES.
>
> I guess we can change both (FOR ALL TABLES and IN SCHEMA) later.
>
That sounds reasonable.
--
With Regards,
Amit Kapila.
