On Tue, Sep 20, 2022 at 2:57 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > On 2022-Sep-20, Amit Kapila wrote: > > > On Mon, Sep 19, 2022 at 8:46 PM Alvaro Herrera <alvhe...@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.