On Mon, 16 Feb 2026 at 10:07, Dilip Kumar <[email protected]> wrote:
>
> I started looking into the patch, I have a few comments/questions
>
> 1.
> + <para>
> + Similarly, if another publication includes the same table with a row
> + filter, but it is also covered by a
> + <literal>FOR ALL TABLES ... EXCEPT</literal> publication, the table is
> + published without applying the row filter, since row filters cannot be
> + specified for <literal>FOR ALL TABLES ... EXCEPT</literal> publications
> + and such publications are therefore treated as having no row filter.
> + </para>
>
> I did not understand what this paragraph is trying to explain? what
> do you mean by it is covered by FOR ALL TABLES ... EXCEPT, does it
> mean the relation is not excluded by EXCEPT?
I have removed these as they follow the existing row filter rules
already mentioned.
> 2.
> + <para>
> + Create a publication that publishes all sequences for synchronization, and
> + all changes in all tables except <structname>users</structname> and
> + <structname>departments</structname>:
> +<programlisting>
> +CREATE PUBLICATION all_sequences_tables_except FOR ALL SEQUENCES, ALL
> TABLES EXCEPT (users, departments);
>
> Do we support EXCEPT SEQUENCES as well or that's for the future?
We don't support sequences, it will be handled later.
> 3.
> +/* Helper: Check syscache for prexcept flag */
> +bool
> +is_relid_published_as_except(Oid relid, Oid pubid)
> +{
>
> IMHO the function name doesn't look great for its purpose, we just
> want to check whether the relid is excluded, published_as_except seems
> confusing, no?
Modified it to is_relation_excluded
> 4.
> +/*
> + * Throws an ERROR if multiple publications with exceptions are found.
> + *
> + * This check is mandatory because logical replication currently does not
> + * support subscribing to multiple publications if more than one of them
> + * uses an exclusion list.
> + */
>
> IMHO explaining the reason why this is not supported or giving
> reference to any other comments where it is already explained would be
> useful.
Updated comments
> 5.
> + /*
> + * If the root ancestor is covered by a FOR ALL TABLES
> + * EXCEPT publication that uses
> + * publish_via_partition_root, we must publish changes via
> + * the root identity.
> + */
> + if (root_published_via_exceptpub)
> + {
> + pub_relid = root_ancestor;
> + ancestor_level = list_length(ancestors);
> + }
>
> I find this comment a bit confusing, what does "covered by a FOR ALL
> TABLES EXCEPT publication" means? It means relation is not in the
> EXCEPT list or in the EXCEPT list?
Updated comments to:
/*
* If the root ancestor is effectively published by an
* ALL TABLES publication with publish_via_partition_root,
* then changes for its partitions must be published
* using the root identity.
*
* This applies even if other publications do not specify
* publish_via_partition_root, provided the root is not
* excluded from that ALL TABLES publication.
*/
Thanks for the comments, these are addressed in the v45 version posted at [1].
Additionally the v45 version also addresses Nisha's comments from [2].
[1] -
https://www.postgresql.org/message-id/CALDaNm11LKbC2epEyHOvD6H_ONqLqhDQk7sXWwcneyhrTbFaTw%40mail.gmail.com
[2] -
https://www.postgresql.org/message-id/CABdArM4KucFdHYPFCcpvpN7_OVnV5rpnDY7Daz7Cjn6ZuN-dkg%40mail.gmail.com
Regards,
Vignesh
