On Wed, 15 Jul 2026 at 18:34, Fujii Masao <[email protected]> wrote: > > On Mon, Jul 13, 2026 at 9:48 PM vignesh C <[email protected]> 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
v2-0001-Fix-sequence-synchronization-for-concurrently-dro.patch
Description: Binary data
