| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
| Cc: | Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Skipping schema changes in publication |
| Date: | 2026-02-17 06:23:00 |
| Message-ID: | CALDaNm3J5V5Z4JT+1owL+bZs4W221E2H8Zkf5YCrA1dAU1BvsA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 16 Feb 2026 at 11:55, David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>
> On Sunday, February 15, 2026, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>>
>> On Mon, Feb 16, 2026 at 8:50 AM vignesh C <vignesh21(at)gmail(dot)com> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-02-17 06:31:53 | Re: Skipping schema changes in publication |
| Previous Message | lakshmi | 2026-02-17 06:11:13 | Re: parallel data loading for pgbench -i |