Re: Logical Replication of sequences - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Logical Replication of sequences
Date
Msg-id CAA4eK1K6Aofz_f6afuL+r2M3GfHBEYQ6-5JO93ph9xZAmYugSA@mail.gmail.com
Whole thread Raw
In response to RE: Logical Replication of sequences  ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>)
List pgsql-hackers
On Thu, Oct 16, 2025 at 4:53 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Thanks! Here is the remaining patches, which addressed all pending comments.
>

Few comments on 0001/0003:
========================
1.
@@ -480,7 +480,9 @@ RemoveSubscriptionRel(Oid subid, Oid relid)
  * leave tablesync slots or origins in the system when the
  * corresponding table is dropped.
  */
- if (!OidIsValid(subid) && subrel->srsubstate != SUBREL_STATE_READY)
+ if (!OidIsValid(subid) &&
+ get_rel_relkind(subrel->srrelid) != RELKIND_SEQUENCE &&
+ subrel->srsubstate != SUBREL_STATE_READY)
  {

Here, why don't we allow sequence rel to be removed? Please add some comments.

2.
/*
  * Get the relations for the subscription.
  *
- * If not_ready is true, return only the relations that are not in a ready
- * state, otherwise return all the relations of the subscription.  The
- * returned list is palloc'ed in the current memory context.
+ * get_tables: get relations for tables of the subscription.
+ *
+ * get_sequences: get relations for sequences of the subscription.
+ *
+ * not_ready:
+ * If getting tables and not_ready is false, retrieve all tables;
+ * otherwise, retrieve only tables that have not reached the READY state.
+ * If getting sequences and not_ready is false, retrieve all sequences;
+ * otherwise, retrieve only sequences that have not reached the READY state.
+ *
+ * The returned list is palloc'ed in the current memory context.
  */
 List *
-GetSubscriptionRelations(Oid subid, bool not_ready)
+GetSubscriptionRelations(Oid subid, bool get_tables, bool get_sequences,
+ bool not_ready)

The existing code comments (without the patch) are good enough for this change.

3. Move catalogs.sgml, alter_subscription.sgml (parts related to 0001)
from 0003 to 0001. Also, see if anything else can be moved.

4.
      <para>
-      Fetch missing table information from publisher.  This will start
+      Fetch missing table information from the publisher.  This will start
       replication of tables that were added to the subscribed-to publications
       since <link linkend="sql-createsubscription">
       <command>CREATE SUBSCRIPTION</command></link> or
       the last invocation of <command>REFRESH PUBLICATION</command>.
      </para>

+     <para>
+      Also, fetch missing sequence information from the publisher.
+     </para>

The second para should be merged into the first one: Fetch missing
table and sequence information from the publisher.

5.
+      </para>
+      <para>
+       State codes for sequences:
+       <literal>i</literal> = initialize,
+       <literal>d</literal> = re-synchronize,
+       <literal>r</literal> = ready
</para></entry>

I think we need to remove the 'd' state from here as the patch 0001
changes always update the sequence state to init.

--
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: shveta malik
Date:
Subject: Re: Logical Replication of sequences
Next
From: Daniel Gustafsson
Date:
Subject: Re: ci: Skip minfree file in the cores_backtrace.sh