Horton

Re: Logical Replication of sequences

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, shveta malik <shveta.malik@gmail.com>, Peter Smith <smithpb2250@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2025-06-03T11:41:13Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Add documentation for sequence synchronization.

  2. Remove unused assignment in CREATE PUBLICATION grammar.

  3. Add seq_sync_error_count to subscription statistics.

  4. Fix few issues in commit 5509055d69.

  5. Add sequence synchronization for logical replication.

  6. Add worker type argument to logical replication worker functions.

  7. Introduce "REFRESH SEQUENCES" for subscriptions.

  8. Refactor logical worker synchronization code into a separate file.

  9. Standardize use of REFRESH PUBLICATION in code and messages.

  10. Add "ALL SEQUENCES" support to publications.

  11. Expose sequence page LSN via pg_get_sequence_data.

  12. Resume conflict-relevant data retention automatically.

  13. Post-commit review fixes for 228c370868.

  14. Generate GUC tables from .dat file

On Thu, May 29, 2025 at 8:09 PM vignesh C <vignesh21@gmail.com> wrote:
>
> These comments are handled in the attached v2025029 version patch.
>

1. The current syntax to publish sequences is:
CREATE PUBLICATION pub1 FOR ALL TABLES, ALL SEQUENCES;

The other alternative could be:
CREATE PUBLICATION pub1 FOR ALL TABLES, SEQUENCES;

I think the syntax proposed by the patch is better because of the
following reasons: (a) The use of ALL before both objects makes the
intent explicit and symmetrical. (b) As we are planning to support FOR
TABLE t1, SEQUENCE s1, so ALL SEQUENCES fit naturally as a counterpart
to ALL TABLES.

Please let me know if anyone thinks otherwise.

2. The 0001 patch has the following commit message:
"Introduce pg_sequence_state function for enhanced sequence
management. This patch introduces a new function, 'pg_sequence_state',
which
allows retrieval of sequence values, including the associated LSN."

It doesn't mention the use of introducing this new function.

Following comments on the 0004 patch
3.
 List *
-GetSubscriptionRelations(Oid subid, bool not_ready)
+GetSubscriptionRelations(Oid subid, bool get_tables, bool get_sequences,
+ bool all_states)

What is the need to change the not_ready flag? Can't not_ready serve the need?

4.
+void
+SetSequence(Oid relid, int64 next, bool is_called, int64 log_cnt)

The argument order seems odd. Isn't it better to keep 'int64 log_cnt'
next to 'int64 next' and then a bool at the end?

5.
+ /*
+ * XXX: If the subscription is for a sequence-only publication, creating
+ * this origin is unnecessary. It can be created later during the ALTER
+ * SUBSCRIPTION ... REFRESH command, if the publication is updated to
+ * include tables.
+ */

Can you explain in comments why creating an origin is unnecessary? The
same is true for a similar comment on slots. If the patch has
explained it somewhere else, then mention a reference to that.

6. ALTER SUBSCRIPTION ... REFRESH PUBLICATION SEQUENCES

Can we move the implementation of the above command to a separate
patch? This is to make 0004 shorter and easier to review.

-- 
With Regards,
Amit Kapila.