On Wed, 11 Mar 2026 at 11:11, Nisha Moond <[email protected]> wrote: > > On Tue, Mar 10, 2026 at 6:19 PM vignesh C <[email protected]> wrote: > > > > Thanks for the comments, these are addressed in the v61 version patch > > attached. > > > Thanks for the patch, I have few comments: > > 1) publicationcmd.c: CheckAlterPublication() > After the recent change to use stmt->for_all_tables, the > "excepttables" parameter is no longer used in this function and can be > removed. > ~~~ > > Couple of minor suggestions: > 2) > + if (stmt->for_all_tables && !superuser()) > + ereport(ERROR, > + errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > + errmsg("must be superuser to define FOR ALL TABLES publication")); > + > > Since this check is in the context of ALTER PUBLICATION, should the > error message instead be: > "must be superuser to alter FOR ALL TABLES publication" > > 3) test comment in publication.sql:1015 - > "-- fail - ADD/DROP EXCEPT table requires superuser privileges" > The test uses only SET, not ADD/DROP, so the comment could be updated to: > > -- fail - modifying EXCEPT table list requires superuser privileges > (I’m also fine with any better alternative.)
These comments are addressed in the v62 version patch attached. Regards, Vignesh
v62-0001-Add-support-for-EXCEPT-TABLE-in-ALTER-PUBLICATIO.patch
Description: Binary data
