On Wed, 11 Mar 2026 at 11:11, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Tue, Mar 10, 2026 at 6:19 PM vignesh C <vignesh21@gmail.com> 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