| 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>, Amit Kapila <amit(dot)kapila16(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-06-30 10:16:58 |
| Message-ID: | CAJpy0uCBkS2MyRzz0WSxg6Ch6Vi9-vNLCWLmEtcu_WwHWP7oiw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Jun 22, 2026 at 6:20 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> Attached updated patches v16. No changes in 0001 to 0003.
>
Thanks Nisha. I have resumed review of patch. I have not goen through
complete patch yet, but please find a few initial comments:
1)
I get this during compilation:
pgoutput.c:2232:45: warning: declaration of ‘except_pubids’ shadows a
previous local [-Wshadow=compatible-local]
2232 | List *except_pubids = NIL;
| ^~~~~~~~~~~~~
pgoutput.c:2100:29: note: shadowed declaration is here
2100 | List *except_pubids;
2)
I am checking the flow:
get_rel_sync_entry-->GetTopMostAncestorInPublication
@@ -2267,7 +2290,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
ancestor = GetTopMostAncestorInPublication(pub->oid,
ancestors,
- &level);
+ &level,
+ except_pubids);
a)
For what all publications, flow will come to above point. I guess it
can come for a explict table pub or a schema pub. For a schmea pub,
except_pubids make sense but not for a explicit-table pub alone.
Please add a comment atop GetTopMostAncestorInPublication call here to
explain
the scenario adn then the need of except_pubids.
b)
We have got except_pubids in the begining of the function as:
except_pubids = GetRelationExcludedPublications(root_relid);
Now inside GetTopMostAncestorInPublication(), we are keeping
'except_pubids' as constant while moving through all the ancestors. We
are fetching GetRelationIncludedPublications and GetSchemaPublications
for each ancestor but are not computing
GetRelationExcludedPublications for each ancestor. It may not be
obvious (for many new readers) why 'except_pubids' is not fetched
again for each ancestor. Please add a comment there (perhaps because
EXCEPT list can not have any partition and can have only root)
3)
The error message below and its associated validation logic are
duplicated in three places:
"cannot appear in both the table list and the EXCEPT clause"
Do you think we could factor this into a small helper function and
reuse it everywhere? That would make the code more modular and
simplify future changes to validation or error messages.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Christoph Berg | 2026-06-30 10:19:21 | Re: [PATCH] Document wal_compression=on |
| Previous Message | John Naylor | 2026-06-30 10:07:13 | Re: [PATCH] Document wal_compression=on |