On Wed, 10 Dec 2025 at 11:21, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Dec 9, 2025 at 11:17 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> > >
> > I have removed the 0001 0002 and 0004 patches for now. Will post them
> > once 0003 patch is RFC.
> > Here is the update patch for "EXCEPT TABLE".
> >
>
> Thanks, I have not looked at new patch yet, but here are few comments
> for v29-003:
>
> 1)
> create_publication.sgml:
> Please add one more example in the example section for EXCEPT using
> 'all tables, and all sequences' after the last existing one. This is
> needed to show that ALL TABLES EXCEPT() and ALL SEQ are still possible
> in single command.
>
> 2)
> + excluded. Optionally, <literal>*</literal> can be specified after the
> + table name to explicitly indicate that descendant tables are excluded.
> + </para>
>
> We may add: This does not apply to a partitioned table, however.
> (this will make it more clear similar to how existing doc has it for
> 'FOR TABLE' clause ). And then start details on partition.
>
> 3)
> When
> + <literal>publish_via_partition_root</literal> is set to
> + <literal>false</literal>, specifying a partitioned table or non-leaf
> + partition has no effect
>
> Can we simply say 'specifying a root partitioned table has no effect'.
> This will make it consistent as the previous sentence also uses the
> same term rather than 'non-leaf'.
>
> 4)
> tab_root is a partitioned table with tab_part_1 and tab_part_2 as its
> partitions.
> In the first case, I receive a WARNING because the user excluded
> tab_part_2 but its data will still be replicated through the root
> table:
>
> postgres=# create publication pub3 for all tables except (tab_part_2)
> WITH (publish_via_partition_root=true);
> WARNING: partition "tab_part_2" will be replicated as
> publish_via_partition_root is "true"
>
> But in the following case, no WARNING is shown:
> postgres=# create publication pub4 for all tables except (tab_root)
> WITH (publish_via_partition_root=false);
> CREATE PUBLICATION
>
> In this scenario, the user has excluded the root table, yet its data
> will still be replicated because publish_via_partition_root = false.
> Should we emit a warning in this case as well? Thoughts?
>
> 5)
> publication_add_relation:
> + if (pub->alltables && pri->except && targetrel->rd_rel->relispartition &&
> + pub->pubviaroot)
>
> Can we please bring both the 'pub' conditions together, as that seems
> more understandable:
> if (pub->alltables && pub->pubviaroot &&...)
>
> 6)
> We have added pubid as argument to GetAllPublicationRelations to
> exclude except-list tables.
> We should change comment atop GetAllPublicationRelations() to indicate
> the same. We should extend
> this existing comment to say about except-list exclusion also.
>
> * If the publication publishes partition changes via their respective root
> * partitioned tables, we must exclude partitions in favor of including the
> * root partitioned tables.
>
Hi Shveta,
I have addressed the above comments and attached the updated patch.
I have also addressed a comment by Peter (comment no. 20 in [1]) which
I missed in the earlier version.
[1]: https://www.postgresql.org/message-id/CAHut%2BPudi%2B9ssBR_Q_Fd29aGEu8s18OyKUGo5w5aKJK-2_c%2B8g%40mail.gmail.com
Thanks,
Shlok Kyal
