Re: Fix publisher-side sequence permission reporting

On Sat, Jun 20, 2026 at 6:14 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sat, Jun 20, 2026 at 7:21 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Sat, Jun 20, 2026 at 2:54 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > >
> > > > Looks great! Thanks for fixing this.
> > >
> > > Thanks for the review! I've pushed the patches.
> > >
> >
> > You seem to have forgotten to update the following comment in
> > pg_get_sequence_data(): "Return all NULLs for missing sequences,
> > sequences for which we lack
> > privileges, other sessions' temporary sequences, ...".
>
> Do you mean that the documentation for pg_get_sequence_data() should
> also mention other sessions' temporary sequences and unlogged sequences
> on standbys, as the comment does? If I've misunderstood your point,
> could you clarify?
>

It is better to update docs for all cases. Sorry, I was wrong in
saying that code comments need an update.

>
> > BTW, if we go
> > by the logic of your proposal, shouldn't one also distinguish other
> > cases as mentioned in the comment quoted by me?
>
> Do you mean that sequencesync.c should also distinguish other
> sessions' temporary sequences and unlogged sequences on standbys, and
> report separate warnings for those cases?
>

Right. I mean to ask if we want to distinguish the lack of privilege
as a separate case then why not others? My opinion on this point is
that improving all these cases (including lack of privileges) together
could be considered as an enhancement for the next version but if you
think this is sort of a must to distinguish one or more cases then we
can do it now as well. However, the reason for doing it now is not
clear to me.

>
> > Also, we might want to consider additional errhint as follows:
>
> Sounds good.
>
>
> > errhint("Grant UPDATE on the sequence to the subscription/sequence
> > owner on the subscriber.")
>
> Wouldn't it be better to drop "sequence" from "subscription/sequence
> owner"? The sequence owner should already have UPDATE privilege on
> the sequence. How about:
>
>     errhint("Grant UPDATE on the sequence to the subscription owner on
> the subscriber.")
>

makes sense.

>
> > errhint("Grant SELECT on the sequence to the replication role on the
> > publisher.")
>
> How about making this a bit more precise?
>
>     errhint("Grant SELECT on the sequence to the role used for the
> replication connection on the publisher.")
>

makes sense.

-- 
With Regards,
Amit Kapila.