On Thu, Jan 29, 2026 at 8:41 PM vignesh C <[email protected]> wrote: > > On Wed, 28 Jan 2026 at 10:46, shveta malik <[email protected]> wrote: > > > > Thank You for the patch. > > > > 1) > > There are certain parts of Approach 3 still present in Approach 1, as > > an example: > > > > 1a) > > + For partitioned tables, only the root partitioned table may be > > specified > > + in <literal>EXCEPT TABLE</literal>. > > > > 1b) > > + /* > > + * Only the topmost ancestor of a partitioned table can be specified > > + * in EXCEPT TABLES clause of a FOR ALL TABLES publication. So fetch > > + * the publications excluding the topmost ancestor only. > > + */ > > + GetRelationPublications(llast_oid(ancestors), NULL, &exceptpuboids); > > + > > > > 1c) > > + /* Check if the partiton is part of EXCEPT list of any publication */ > > + GetRelationPublications(RelationGetRelid(attachrel), NULL, > > &except_pubids); > > + if (except_pubids != NIL) > > + ereport(ERROR, > > + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > > + errmsg("cannot attach relation \"%s\" as partition because it is > > part of EXCEPT list in publication", > > + RelationGetRelationName(attachrel)))); > > + > > > > Overall, please take a diff of v35 and v37 to find such parts and > > please correct these and others (if any). > > > > 2) > > Also I don't think if below is correct statement for Approach 1: > > > > + * 2. For a partition, if the topmost ancestor is part of > > + * the EXCEPT TABLE list, we don't publish it. > > > > Even if any ancestor is part of EXECPT list (not only top most) we > > should not publish that partition, isn't it? > > > > 3) > > I tried a scenario and found that incremental replication is not > > working correctly. Attached the failing test as Approach1_v37_fail.txt > > > > Once these basic things are corrected, I can review further. > > These comments are addressed in the v38 version patch attached. > Currently the approach-3 changes is present separately in > v38-0002-Restrict-EXCEPT-TABLE-to-root-partitioned-tables-apporach-3.patch > which can be applied on top of > v38-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch. > Similarly the approach-1 changes is present separately in > v38-0002-handle-EXCEPT-TABLE-correctly-with-partitioned-approach-1.patch > which can be applied on top of > v38-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch. > Currently few of the query are logged as LOG messages, I will reduce > the log level for these queries once few rounds of review are > completed on the queries. >
I was trying to test Approach 1 and haven’t fully reviewed or validated it yet. A few initial observations: postgres=# CREATE PUBLICATION pub1 for all tables EXCEPT(tab_part_1,tab_part_2_p2) WITH (PUBLISH_VIA_PARTITION_ROOT=true); WARNING: partition "tab_part_1" might be replicated as publish_via_partition_root is "true" WARNING: partition "tab_part_2_p2" might be replicated as publish_via_partition_root is "true" CREATE PUBLICATION postgres=# CREATE PUBLICATION pub1 for all tables EXCEPT(tab_part_1,tab_part_2_p2) WITH (PUBLISH_VIA_PARTITION_ROOT=false); WARNING: partitioned table "tab_part_1" might be replicated as publish_via_partition_root is "false" CREATE PUBLICATION IIUC, these Warnings are not expected in Approach1. thanks Shveta
