Re: Logical Replication of sequences

On Thu, 1 Aug 2024 at 09:26, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Mon, Jul 29, 2024 at 4:17 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > Thanks for reporting this, these issues are fixed in the attached
> > v20240730_2 version patch.
> >
>
> Thanks for addressing the comments. Please find few comments on patch001 alone:
>
> Potential Bug:
> 1) 'last_value' returned by pg_sequence_state() is wrong initially.
>
> postgres=# create sequence myseq5;
> CREATE SEQUENCE
>
> postgres=# select * from pg_sequence_state('myseq5');
>  page_lsn  | last_value | log_cnt | is_called
> -----------+------------+---------+-----------
>  0/1579C78 |          1 |       0 | f
>
> postgres=# SELECT nextval('myseq5') ;
>  nextval
> ---------
>        1
>
> postgres=# select * from pg_sequence_state('myseq5');
>  page_lsn  | last_value | log_cnt | is_called
> -----------+------------+---------+-----------
>  0/1579FD8 |          1 |      32 | t
>
>
> Both calls returned 1. First call should have returned NULL.

I noticed the same behavior for selecting from a sequence:
postgres=# select * from myseq5;
 last_value | log_cnt | is_called
------------+---------+-----------
          1 |       0 | f
(1 row)

postgres=# select nextval('myseq5');
 nextval
---------
       1
(1 row)

postgres=# select * from myseq5;
 last_value | log_cnt | is_called
------------+---------+-----------
          1 |      32 | t
(1 row)

By default it shows the last_value as the start value for the
sequence. So this looks ok to me.

> 2)
> func.sgml:
> a) pg_sequence_state : Don't we need to give input arg as regclass
> like we give in  nextval,setval etc?
> b) Should 'lsn' be changed to 'page_lsn' as returned in output of
> pg_sequence_state()

Modified

>
> 3)
> read_seq_tuple() header says:
>  * lsn_ret will be set to the page LSN if the caller requested it.
>  * This allows the caller to determine which sequence changes are
>  * before/after the returned sequence state.
>
> How, using lsn which is page-lsn and not sequence value/change lsn,
> does the user interpret if sequence changes are before/after the
> returned sequence state? Can you please elaborate or amend the
> comment?

I have added this to pg_sequence_state function header in 003 patch as
the subscriber side changes are present here, I felt that is more apt
to mention. This is also added in sequencesync.c file header.

The attached v20240805_2 version patch has the changes for the same.

Regards,
Vignesh