On Tue, Mar 10, 2026 at 11:16 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Mar 10, 2026 at 10:25 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
>
> > * Also, the superuser check should be earlier in the code, say just
> > after following existing check:
> > if ((stmt->action == AP_AddObjects || stmt->action == AP_SetObjects) &&
> > schemaidlist && !superuser())
> > ereport(ERROR,
> > (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> > errmsg("must be superuser to add or set schemas")));
> >
>
> I too thought about it, but then it seems the current order of check
> gives better error (IMO). As an example consider where pub1 is a
> publication for explicit tables, then doing 'ALTER PUB SET ALL TABLES'
> on it should give the error that 'pub1 is not for ALL TABLES' rather
> than 'must be superuser'. Because even if ownership was correct, it
> was not going to work.
>
> Currently:
>
> postgres=> alter publication pub1 set all tables;
> ERROR: publication "pub1" is not defined as FOR ALL TABLES
>
> And for pub2 which is for all tables, we get user error:
>
> postgres=> alter publication pub2 set all tables;
> ERROR: must be superuser to add or drop except tables
>
> The errors in both cases seem correct to me. What do you think?
>
I still feel superuser check should be first and the user should get
that when it tries to use FOR ALL TABLES. Also, the better error
message for the same would be: "must be superuser to define FOR ALL
TABLES publication"
--
With Regards,
Amit Kapila.
