On Mon, 16 Feb 2026 at 11:55, David G. Johnston
<[email protected]> wrote:
>
> On Sunday, February 15, 2026, Dilip Kumar <[email protected]> wrote:
>>
>> On Mon, Feb 16, 2026 at 8:50 AM vignesh C <[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 concur the wording is tough to parse. “Covered” is probably not a good
> word to use. Stick with either included or excluded. In this case, the
> point being communicated is if two publications include a table and one
> doesn’t have a where clause a combining subscription will consider the where
> clause to be a constant true when combining the where clauses using OR.
>
> This wording basically already exists in create subscription. This paragraph
> and the preceding one, which discuss “if another publication exists”, seems
> out of place in the create publication documentation. This page should be
> restricted to only those behaviors that manifest when dealing with a single
> publication, detailing what it produces.
I have removed these as they follow the existing row filter rules
already mentioned.
>>
>>
>> 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.
>
>
> The word “exceptions” needs to be turned into “except clauses” or
> “exclusions”.
Modified
>>
>>
>> 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?
>
>
>
> root_published_via_exceptpub … shouldn’t this just be
> “root_published_via_alltables? It’s the all table clause that prohibits
> "where" and accepts "except". IOW, it's sufficient to drop mention of except
> because the nature of the boolean means that 'false == except'.
>
> Something Like:
> When dealing with multiple publications in a subscription, a
> publish_via_partition_root attribute on an ALL TABLES publication causes the
> system to direct the data for all subscribed partitions to the partition
> root, even if other publications do not specify publish_via_partition_root
> (so long as the partition root in question hasn't been excluded from the ALL
> TABLES publication).
Updated with few changes. Let me know if it did not convey the same
meaning, I will use your exact wording.
> Note: all tables + publish_via_partition_root = false comes to mind here but
> I'm unable to dive into the sidebar at the moment. It seems like
> root_published_via_alltables being false covers that case sufficiently though
> since in effect the root isn't published in that scenario.
>
> + * If the relation is a partition, check whether the current
> + * relation or any of the ancestors is included in the EXCEPT
> + * TABLE list. If so, do not publish the change. This is done
> + * irrespective of pubviaroot setting.
>
> I don't understand the motivation for pointing out pubviaroot here. Of
> course a table that is not published doesn't care what 'pub'viaroot says.
Removed this to avoid confusion.
> + * If the top-most ancestor is covered by a 'FOR ALL TABLES
> + * EXCEPT' publication, we don't need to track per-relation
> + * metadata here. These publications apply globally and do not
> + * support row filters or column lists that would require
> + * specific tracking for this partition.
>
> Suggestion:
> /* Partitions whose top-most ancestor is being published via an ALL TABLES
> publication need not be individually published via any other publication.
> Repeated occurrences of a partition take the least restricted definition,
> which the ALL TABLES publication always provides. I.e., all columns, all
> rows, no children left behind. */
> if (root_published_via_alltables)
> continue;
Modified, I have left the no children left behind as the child
specified in the except clause will be skipped if it is not included
via another publication.
> The last two sentences are inferrable (from the definition/description of the
> rules of publication combining) and probably should be excluded.
>
> If the root is either "except'ed or there is no "publish via partition root"
> this is false and the subsequent lappend happens per usual. That seems
> correct.
I have removed the newly added comment i.e.:
* If the relation is part of EXCEPT TABLE list of a publication,
* the 'publish' variable is already set to false.
I have retained the other one as it was an existing comment.
> + * explicitly specified in the EXCEPT claus of the given publication.
> s.b., "clause"
Fixed this.
Thanks for the comments, these are addressed in the v45 version posted at [1].
[1] -
https://www.postgresql.org/message-id/CALDaNm11LKbC2epEyHOvD6H_ONqLqhDQk7sXWwcneyhrTbFaTw%40mail.gmail.com
Regards,
Vignesh
