Re: Skipping schema changes in publication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Skipping schema changes in publication
Date: 2026-03-10 06:17:32
Message-ID: CAA4eK1+hOujZNuczYg6xVMMB0H96Co3TuuuKRvUjNNMgQ_xz3g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 10, 2026 at 11:16 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Mar 10, 2026 at 10:25 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-03-10 06:18:58 Re: Add support for EXTRA_REGRESS_OPTS for meson
Previous Message Japin Li 2026-03-10 06:12:23 Re: Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)