Re: Support EXCEPT for TABLES IN SCHEMA publications

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

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.

3)
I have yet to reveiw pub_contains_invalid_column, but I have a initial
understanding, that it will be similar to the case of
pub_rf_contains_invalid_column. Please verify on your end.

[1]: https://www.postgresql.org/message-id/CAJpy0uAuL7SqowL7T6HCELuVUr5ubwoGsjV1SQQRCXGme5L%3DpA%40mail.gmail.com

thanks
Shveta

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2026-07-08 09:05:40 Re: Wrong query result w/ propgraph single lateral col reference
Previous Message Jelte Fennema-Nio 2026-07-08 08:47:52 Re: Can we get rid of TerminateThread() in pg_dump?