| 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.
thanks
Shveta
| 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? |