Re: Skipping schema changes in publication

On Tue, 3 Feb 2026 at 22:23, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 2 Feb 2026 at 17:18, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> >
> > I reviewed v38-0002-handle-EXCEPT-TABLE-correctly-with-partitioned-approach-1.patch
> > patch. Here are my comments:
> >
> > 4. While testing, I noticed that the new query introduced in tablesync
> > can be invoked for "FOR TABLE". SHould we only call it for "ALL
> > TABLES" publications?
> > + if (server_version >= 190000 && !is_partition &&
> > + lrel->relkind == RELKIND_PARTITIONED_TABLE)
> > + {
> > + resetStringInfo(&cmd);
> > +
> > + /*
> > + * This query recursively traverses the inheritance (partition) tree
> > + * starting from the given table OID and determines which leaf
> > + * relations should be included for replication. Exclusion propagates
> > + * from parent to child, and a relation is also treated as excluded if
> > + * it is explicitly marked with prexcept = true in pg_publication_rel
> > + * for the specified publications. The final result returns only
> > + * non excluded leaf relations.
> > + */
> > Test:
> > Create publication for table sc1.t1 using (publish_via_partition_root
> > = true) and create subscription on it. In subscriber logs we can see
> > the logs for the new query.
> > sc1.t1 has the same structure as in comment 1.
>
> We will not know if it is a table publication or all tables
> publication from here. Also there can be a possibility of multiple
> publications. We will invoke it and handle it from the publisher to
> return appropriate tables.
>
> Thanks for the review and detailed comments. All remaining feedback
> has been addressed in the attached v39 patch.
>
> In addition, the SQL query previously used to compute the set of
> effective tables has been replaced with a C implementation. The SQL
> approach had become increasingly complex and difficult to reason
> about, especially as more publication combinations were added.
> Implementing this logic in C significantly improves readability and
> maintainability, and makes it easier to handle complex scenarios, such
> as:
> a) Multiple publications where one publication has no EXCEPT tables
> and another does.
> b) Multiple publications where one publication is an ALL TABLES
> publication with EXCEPT, while another is a table-specific
> publication.
> c) Multiple publications where none of the publications define any
> EXCEPT tables.
>
> Peter's comments from [1] and Shveta's comments form [2] will be
> addressed in the next version.
> [1] - https://www.postgresql.org/message-id/CAHut%2BPsiWwmNSuCXTWM0iPDm3yGskLts-fukELTB__rbBids-A%40mail.gmail.com
> [2] - https://www.postgresql.org/message-id/CAJpy0uAOvtMBP-oV9Tgoznt5-UsE2dzAjZW3eJmgKcU-X-vEzg%40mail.gmail.com
>
Hi Vignesh,

Thanks for providing the updated patch.
While testing the patch I have found a small bug:
```
+ if (am_partition)
{
List *ancestors = get_partition_ancestors(relid);

- pub_relid = llast_oid(ancestors);
- ancestor_level = list_length(ancestors);
+ foreach_oid(ancestor, ancestors)
+ GetRelationPublications(ancestor, NULL, &exceptpubids);
+
+ if (pub->pubviaroot)
+ {
+ pub_relid = llast_oid(ancestors);
+ ancestor_level = list_length(ancestors);
+ }
}

GetRelationPublications(pub_relid, NULL, &exceptpubids);
```
Here at last "GetRelationPublications" should be called for 'relid',
instead of 'pub_relid'. The call for function
"GetRelationPublications" can be skipped for table which is not root
partitioned but is specified explicitly in the EXCEPT TABLE list.
Test:
The partition structure is as:
sc1.t1
- sc1.child1
- sc1.child1_1
- sc1.child1_2
- sc1.child2

Suppose we create two publications.
CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT TABLE (sc1.child2) WITH
(publish_via_partition_root = true);
CREATE PUBLICATION pub2 FOR TABLE sc1.child1_1 WITH
(publish_via_partition_root = true);
And on subscriber node we create a subscriber as:
CREATE SUBSCRIPTION sub1 CONNECTION 'host=localhost port=5432
dbname=postgres' PUBLICATION pub1, pub2;

Ideally changes of 'sc1.child2' should not be replicated, but
increment sync replicates the data for 'sc1.child2'.
I have fixed this in the latest version of patch.
I have also added extended tests for this patch as 0003 patch. The
last test in the file tests the above issue.

I have also addressed Peter's comment in [1].
For comment:
> 3.
> + appendPQExpBuffer(query, "ONLY %s", fmtQualifiedDumpable(tbinfo));
>
> I think that unconditionally choosing "ONLY" here may not be the right
> thing to do, particularly when the excluded table is a partitioned
> table. (e.g. this is related to off-list discussions about how to
> EXCEPT partition tables).
The ONLY clause has no effect with partition tables but ONLY clause is
required for inherited tables.
And ALTER PUBLICATION ... ADD TABLE also uses ONLY clause
unconditionally. So, I think this behaviour is consistent with ALTER
PUBLICATION ... ADD TABLE
According to the latest discussion, I think we are proceeding with
Approach 1 in [2]. And EXCEPT TABLE is independent of ONLY. So, I
think we can keep it.

[1]: https://www.postgresql.org/message-id/CAHut+PvmCPdbScDoGV3jX42STm2F3DUWbj7nnbn5Y_zs6w8XWA@mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAJpy0uD81HRrMYr7S-6AV4W2PtbGKM-nf2D89zsoMHJ9jZssUg@mail.gmail.com

Thanks,
Shlok Kyal