Re: Skipping schema changes in publication

On Fri, 20 Feb 2026 at 18:17, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> Hi,
>
> On Fri, Feb 20, 2026 at 2:08 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Thu, 19 Feb 2026 at 16:06, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Thu, Feb 19, 2026 at 10:13 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> > > >
> > > > Thanks for reviewing the patch. I have addressed the remaining
> > > > comments in the v46 patch..
> > > >
> > >
> > > Can we think of some ideas to split this patch? One possibility is
> > > that in the first patch we give an ERROR, if a non-root partitioned
> > > table is present in EXCEPT Clause. I see that a lot of code is written
> > > to handle partitions in EXCEPT clause. I feel such a split will make
> > > code easier to review and validate.
> >
> > Split it accordingly.
> >
> > > Few other comments:
> > > =================
> > > 1.
> > >   if (stmt->for_all_tables)
> > >   {
> > > + /* Process EXCEPT table list */
> > > + if (relations != NIL)
> > > + {
> > > + Assert(rels != NIL);
> > > + PublicationAddTables(puboid, rels, true, NULL);
> > > + }
> > > +
> > >   /*
> > >   * Invalidate relcache so that publication info is rebuilt. Sequences
> > >   * publication doesn't require invalidation, as replica identity
> > > CacheInvalidateRelcacheAll();
> > >
> > > Here, the relations listed in the except table list will be
> > > invalidated twice, once inside
> > > PublicationAddTables->publication_add_relation, and second time via
> > > CacheInvalidateRelcacheAll. Can we avoid that by adding a parameter to
> > > PublicationAddTables?
> >
> > Fixed this
> >
> > > 2.
> > > - root_relids = GetPublicationRelations(pubform->oid,
> > > -   PUBLICATION_PART_ROOT);
> > > + root_relids = GetIncludedRelationsByPub(pubform->oid,
> > > + PUBLICATION_PART_ROOT);
> > >
> > > To follow the previous function naming pattern, can we rename
> > > GetIncludedRelationsByPub to GetIncludedPublicationRelations? If we
> > > agree to this then rename the excluded version of the function as
> > > well.
> >
> > Modified
> >
> > > 3.
> > > +/*
> > > + * Return the list of relation Oids for a publication.
> > > + *
> > > + * For a FOR TABLE publication, this returns the list of relations explicitly
> > > + * included in the publication.
> > > + *
> > > + * Publications declared with FOR ALL TABLES/SEQUENCES should use
> > > + * GetAllPublicationRelations() to obtain the complete set of tables/sequences
> > > + * covered by the publication.
> > > + */
> > > +List *
> > > +GetIncludedRelationsByPub(Oid pubid, PublicationPartOpt pub_partopt)
> > >
> > > This is equivalent to the existing function GetPublicationRelations()
> > > which has more precise comments atop. We can use the same comments
> > > unless there is any functionality difference.
> >
> > Updated it
> >
> > > 4.
> > > --- a/src/backend/catalog/pg_publication.c
> > > +++ b/src/backend/catalog/pg_publication.c
> > > @@ -29,6 +29,7 @@
> > >  #include "catalog/pg_publication.h"
> > >  #include "catalog/pg_publication_namespace.h"
> > >  #include "catalog/pg_publication_rel.h"
> > > +#include "catalog/pg_subscription.h"
> > >
> > > It appears odd to include pg_subscription.h in the publication code.
> > > Is there a reason for the same? If not then avoid it.
> >
> > This was required because we now started using GetPublicationsStr. I
> > have moved GetPublicationsStr to logical.c from pg_subscription which
> > is common to both pub and sub.
> >
> > > Apart from above, a few cosmetic changes are attached.
> >
> > Merged them.
> >
> > The attached v47 version patch has the changes for the same.
> >
>
> SIGABRT with normal tables:
> --------------------------------------
>
> ashutosh@test=# select
> pg_get_publication_effective_tables('t1'::regclass,
> '{all_pub}'::text[]);
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> Since the function is user-facing, I think we should have test coverage for it.
>
Hi Ashutosh,
Thanks for reviewing the patch.

We have fixed this issue.
This function is not user facing. We have a similar function
'pg_get_publication_tables'. We do not have documentation and test
coverage for this function.
I think 'pg_get_publication_effective_tables' is a similar function,
so we should not add documentation or tests for it.

We have also addressed the comments in [1]. I have also done minor
code and comment modifications
I have also modified the error message as suggested by Shveta in [2].
Attached the latest v48 patch.

[1]: https://www.postgresql.org/message-id/CAE9k0P=SfawUYBoSmK9Zxwey774OJw5g4Gb7+fA3CQ22j83a7Q@mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAJpy0uAHr8=YUKSyEBs8G4P0zGGb6mcUr-YJdB56tdYMF9yoKA@mail.gmail.com

Thanks,
Shlok Kyal