Re: Logical Replication of sequences

On Tue, Jan 20, 2026 at 2:24 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Tue, 20 Jan 2026 at 11:00, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Tue, 20 Jan 2026 at 09:47, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Jan 19, 2026 at 6:36 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > >
> > > > pg_get_sequence_data() internally uses try_relation_open() rather than
> > > > relation_open(). As a result, if the target sequence no longer exists
> > > > at the time of access, the function does not raise an error and
> > > > instead returns NULLs for the sequence state columns. The sequence
> > > > sync worker code previously assumed these columns to be non NULL and
> > > > asserted accordingly. This assumption does not hold in the presence of
> > > > concurrent DDL. The patch updates the sequence sync logic to
> > > > explicitly check for NULL values returned from pg_get_sequence_data().
> > > > If any of the required sequence state fields are NULL, the sequence
> > > > sync worker skips processing that sequence to identify and report the
> > > > missing sequences. The attached patch has the changes for the same.
> > > >
> > >
> > > - seqinfo_local->last_value = DatumGetInt64(slot_getattr(slot, ++col, &isnull));
> > > - Assert(!isnull);
> > > + /*
> > > + * If the sequence was dropped concurrently, pg_get_sequence_data() can
> > > + * return NULLs.
> > > + */
> > > + datum = slot_getattr(slot, ++col, &isnull);
> > > + if (isnull)
> > > + return COPYSEQ_SKIPPED;
> > > + seqinfo_local->last_value = DatumGetInt64(datum);
> > >
> > > - seqinfo_local->is_called = DatumGetBool(slot_getattr(slot, ++col, &isnull));
> > > - Assert(!isnull);
> > > + datum = slot_getattr(slot, ++col, &isnull);
> > > + if (isnull)
> > > + return COPYSEQ_SKIPPED;
> > > + seqinfo_local->is_called = DatumGetBool(datum);
> > >
> > > Is there a case where the first one (last_value) is non-null but later
> > > can be null? If not, then I think it is better to retain assert for
> > > other cases.
> >
> > That is not possible. Updated accordingly with slight change to
> > comment. The attached patch has the changes for the same.
>
> During further testing, I identified an issue where the code could not
> reliably distinguish whether a sequence was dropped on the subscriber
> or on the publisher. A sequence dropped on the subscriber should be
> treated as a successful case, since the only viable action is to skip
> it, whereas a missing sequence on the publisher should be treated as
> an error. This distinction has now been handled correctly and fixed in
> the updated version.
>

Thanks for the patch. Pushed.

--
With Regards,
Amit Kapila.