Re: Support EXCEPT for TABLES IN SCHEMA publications

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
Date: 2026-07-08 09:28:15
Message-ID: CAJpy0uCY0FVmHCLprk0vhLiYwPHoKvv71os1aD1syzPzWpaDng@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 8, 2026 at 2:22 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> Nisha, the patch (testing wise) is okay, but some code-optimization
> might help. I am stuck on reveiw of GetTopMostAncestorInPublication()
> and all its callers.
>
> 1)
> caller 1: pub_rf_contains_invalid_column():
>
> It is not understood why pub_rf_contains_invalid_column() does not
> pass 'exceptPubids' to GetTopMostAncestorInPublication(). Perhaps a
> comment will help. IIUC, a parittion (for which we are trying to
> verify row-filter expression) can not co-exist in a subscription where
> its ROOT is already excluded. This assumption will be true once you
> address the issue of CREATE SUB (in [1]) sent in my previous email.
> After that, this will still need a comment.
>
> 2)
> Both caller 2 (get_rel_sync_entry) and caller 3
> (is_table_publishable_in_publication) are trying to see if partition
> should be considered as included in publication and thus are trying to
> see if ROOT is excluded. But the implementation is very different.
>
> The get_rel_sync_entry() does that by passing except_pubids to
> GetTopMostAncestorInPublication(). This except_pubids was computed for
> topmost ROOT of the parition in the caller and then it skips checking
> pg_pub_namepspace if given pubid is part of except_pubids arguement.
>
> While is_table_publishable_in_publication() does that by selecting
> topmost ancestor incuded in a given pubid using
> GetTopMostAncestorInPublication(), without consulting/computing
> except_pubids. And then later checks if the ancestor returned by
> GetTopMostAncestorInPublication() is in pg_publicaiton_rel with
> 'prexcept' true.
>
> IMO, GetTopMostAncestorInPublication() should itself be inclusive of
> logic where it filters out the table (does not return it as result) if
> ROOT is excluded. And even we should not be passing an argument for
> that (this is my initial thought). By making such a logic, we need not
> to bother about all
> the callers to see if caller has correct logic to deal with output of
> GetTopMostAncestorInPublication(). Can you think on this line and
> check the feasibility.
>

I thought more on this. Do you think we can do this?

GetTopMostAncestorInPublication() is already accepting a list of
ancestors in a ordered fashion, root at the end. We get root from this
ancestor list, check if root is in pg_publication_rel with
except=true. If so, we skip the rest of the logic and return
InvalidOid from GetTopMostAncestorInPublication().

The 'except_pubids' argument is not required. Callers need not to have
special logic to send this arguement or to have extra-processing on
output as done by is_table_publishable_in_publication() currently.
Let me know if I have missed anything.

thanks
Shveta

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hans Buschmann 2026-07-08 10:01:32 AW: Increased SECURITY RISCS from omitting some compikler options when building PG with meson; e.g. -fcf-protection=full
Previous Message Fujii Masao 2026-07-08 09:19:44 Re: Fix publisher-side sequence permission reporting