Re: Logical Replication of sequences
Dilip Kumar <dilipbalaut@gmail.com>
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Doc: Add documentation for sequence synchronization.
- 55cefadde874 19 (unreleased) landed
-
Remove unused assignment in CREATE PUBLICATION grammar.
- bfb7419b0bbe 19 (unreleased) landed
-
Add seq_sync_error_count to subscription statistics.
- f6a4c498dcf6 19 (unreleased) landed
-
Fix few issues in commit 5509055d69.
- 5a4eba558aa7 19 (unreleased) landed
-
Add sequence synchronization for logical replication.
- 5509055d6956 19 (unreleased) landed
-
Add worker type argument to logical replication worker functions.
- 3e8e05596a02 19 (unreleased) landed
-
Introduce "REFRESH SEQUENCES" for subscriptions.
- f0b3573c3aac 19 (unreleased) landed
-
Refactor logical worker synchronization code into a separate file.
- 41c674d2e31e 19 (unreleased) landed
-
Standardize use of REFRESH PUBLICATION in code and messages.
- 2436b8c047ff 19 (unreleased) landed
-
Add "ALL SEQUENCES" support to publications.
- 96b378497346 19 (unreleased) landed
-
Expose sequence page LSN via pg_get_sequence_data.
- b93172ca59f4 19 (unreleased) landed
-
Resume conflict-relevant data retention automatically.
- 0d48d393d465 19 (unreleased) cited
-
Post-commit review fixes for 228c370868.
- 1f7e9ba3ac4e 19 (unreleased) cited
-
Generate GUC tables from .dat file
- 63599896545c 19 (unreleased) cited
On Thu, Jul 17, 2025 at 4:52 PM vignesh C <vignesh21@gmail.com> wrote: > > On Wed, 16 Jul 2025 at 11:15, Dilip Kumar <dilipbalaut@gmail.com> wrote: > > > > On Mon, Jul 14, 2025 at 10:03 AM vignesh C <vignesh21@gmail.com> wrote: > > > > > > On Fri, 11 Jul 2025 at 14:26, shveta malik <shveta.malik@gmail.com> wrote: > > > > > > I have picked this up again for final review, started with 0001, I > > think mostly 0001 looks good to me, except few comments > > > > 1. > > + lsn = PageGetLSN(page); > > + last_value = seq->last_value; > > + log_cnt = seq->log_cnt; > > + is_called = seq->is_called; > > + > > + UnlockReleaseBuffer(buf); > > + sequence_close(seqrel, NoLock); > > + > > + /* Page LSN for the sequence */ > > + values[0] = LSNGetDatum(lsn); > > + > > + /* The value most recently returned by nextval in the current session */ > > + values[1] = Int64GetDatum(last_value); > > + > > > > I think we can avoid using extra variables like lsn, last_value etc > > instead we can directly copy into the value[$] as shown below. > > > > values[0] = LSNGetDatum(PageGetLSN(page)); > > values[1] = Int64GetDatum(seq->last_value); > > ... > > UnlockReleaseBuffer(buf); > > sequence_close(seqrel, NoLock); > > Modified > > > 2. > > + <para> > > + Returns information about the sequence. <literal>page_lsn</literal> is > > + the page LSN of the sequence, <literal>last_value</literal> is the > > + current value of the sequence, <literal>log_cnt</literal> shows how > > + many fetches remain before a new WAL record must be written, and > > + <literal>is_called</literal> indicates whether the sequence has been > > + used. > > > > Shall we change 'is the page LSN of the sequence' to 'is the page LSN > > of the sequence relation' > > Modified > > > And I think this field doesn't seem to be very relevant for the user, > > although we are exposing it because we need it for internal use. > > Maybe at least the commit message of this patch should give some > > details on why we need to expose this field. > > Updated commit message > > The attached v20250717 version patch has the changes for the same. Thanks, 0001 looks fine after this, I will share the feedback for other patches. -- Regards, Dilip Kumar Google