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.)
--
Thanks,
Nisha
