Re: Skipping schema changes in publication

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, vignesh C <vignesh21@gmail.com>, "David G. Johnston" <david.g.johnston@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Peter Smith <smithpb2250@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, YeXiu <1518981153@qq.com>, Ian Lawrence Barwick <barwick@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-02-19T10:36:46Z
Lists: pgsql-hackers

Attachments

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.

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?

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.

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.

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.

Apart from above, a few cosmetic changes are attached.

-- 
With Regards,
Amit Kapila.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix miscellaneous issues in EXCEPT publication clause.

  2. Change syntax of EXCEPT TABLE clause in publication commands.

  3. Add support for EXCEPT TABLE in ALTER PUBLICATION.

  4. Allow table exclusions in publications via EXCEPT TABLE.

  5. Add wait_for_subscription_sync for TAP tests.