Re: Fix publisher-side sequence permission reporting

On Wed, 15 Jul 2026 at 18:34, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Mon, Jul 13, 2026 at 9:48 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > I've attached a patch to handle this case by treating a 'NULL'
> > privilege value as indicating that the sequence no longer exists and
> > reporting it through the existing "missing sequence on publisher"
> > path.
> > Thoughts?
>
> Thanks for the patch!
> It looks good to me overall. I just have a couple of minor comments.
>
>
> remote_has_select_priv = DatumGetBool(slot_getattr(slot, ++col, &isnull));
> - Assert(!isnull);
> + if (isnull)
> + return COPYSEQ_SKIPPED;
>
> Isn't it be better to call DatumGetBool() after checking isnull,
> similar to how we handle seqinfo_local->last_value?
>
>
> - * The remote sequence state can be NULL if the publisher lacks the
> - * required privileges or if the sequence was dropped concurrently after
> - * it was identified in the catalog snapshot (see pg_get_sequence_data()).
>
> I think it's worth keeping this comment. How about something like
> applying the following based on your patch?
>
> ----------------------------------------------
> /*
> * has_sequence_privilege() itself returns NULL, rather than false, when
> * the sequence has been dropped concurrently after it was identified in
> - * the catalog snapshot (see has_sequence_privilege_id()). Treat that the
> - * same as the concurrent-drop case detected via the data columns below,
> - * rather than misreporting it as an insufficient-privilege failure.
> + * the catalog snapshot (see has_sequence_privilege_id()). Treat that as
> + * a missing sequence on the publisher.
> */
> - remote_has_select_priv = DatumGetBool(slot_getattr(slot, ++col, &isnull));
> + datum = slot_getattr(slot, ++col, &isnull);
> if (isnull)
> return COPYSEQ_SKIPPED;
> + remote_has_select_priv = DatumGetBool(datum);
>
> + /*
> + * The remote sequence state can be NULL if the publisher lacks the
> + * required privileges or if the sequence was dropped concurrently after
> + * it was identified in the catalog snapshot (see pg_get_sequence_data()).
> + */
> datum = slot_getattr(slot, ++col, &isnull);
> if (isnull)
> return remote_has_select_priv ? COPYSEQ_SKIPPED :
> ----------------------------------------------

Your suggested changes look good to me, here is an updated v2 version
patch with the changes. I have also verified that Zsolt's test at [1]
also passes with the attached patch.
[1] - https://www.postgresql.org/message-id/CAN4CZFPfmF%3DEpT-9YOKYYzptPt5Q7D5xV-v_RrncbfsSZOAcDw%40mail.gmail.com

Regards,
Vignesh