public inbox for [email protected]  
help / color / mirror / Atom feed
From: Fujii Masao <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Tristan Partin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix publisher-side sequence permission reporting
Date: Sat, 20 Jun 2026 21:43:59 +0900
Message-ID: <CAHGQGwEA6PMAE0aMx4MFqj9d+q2Q835gfUyTSJjAkoMwtWB77A@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1JOo0aJRhFHNWpj3hMwaTtNOopY34f1Lh_QD=z=+DrzWQ@mail.gmail.com>
References: <CAHGQGwGNTaXnBKUV510_P1KwhdbHT+kgZ4zU5njBHy7nCqdhzg@mail.gmail.com>
	<[email protected]>
	<CAHGQGwE95GLD7RZr2g=zTHKJRafKACmKouhb4TTVH155Hy8zKA@mail.gmail.com>
	<[email protected]>
	<CAHGQGwFLXCAxKEyvvaTLadHZ8k4DFmjJopnN+Vy-NjtfKoM7dw@mail.gmail.com>
	<CAA4eK1JOo0aJRhFHNWpj3hMwaTtNOopY34f1Lh_QD=z=+DrzWQ@mail.gmail.com>

On Sat, Jun 20, 2026 at 7:21 PM Amit Kapila <[email protected]> wrote:
>
> On Sat, Jun 20, 2026 at 2:54 PM Fujii Masao <[email protected]> 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?


> 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?


> 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.")


> 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.")

Regards,

-- 
Fujii Masao






view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Fix publisher-side sequence permission reporting
  In-Reply-To: <CAHGQGwEA6PMAE0aMx4MFqj9d+q2Q835gfUyTSJjAkoMwtWB77A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox