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

Attachment: v62-0001-Add-support-for-EXCEPT-TABLE-in-ALTER-PUBLICATIO.patch
Description: Binary data

Reply via email to