Re: Logical Replication of sequences

I just reviewed 0001 and got a few comments wrt code comments. I may find some time to review 0002 and 0003 next week.

> On Oct 16, 2025, at 19:23, Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> <v20251016-0003-Documentation-for-sequence-synchronization.patch><v20251016-0002-New-worker-for-sequence-synchronization-du.patch><v20251016-0001-Introduce-REFRESH-SEQUENCES-for-subscripti.patch>

1 - 0001 - pg_subscription.c
```
+ /*
+ * Skip sequence tuples. If even a single table tuple exists then the
+ * subscription has tables.
+ */
+ if (get_rel_relkind(subrel->srrelid) == RELKIND_RELATION ||
+ get_rel_relkind(subrel->srrelid) == RELKIND_PARTITIONED_TABLE)
+ {
+ has_subrels = true;
+ break;
+ }
```

The comment "If even a single table tuple exists then the subscription has tables” sounds redundant. I know it’s inherited from the old code, but now, with the “break” you newly added, the code logic is simple and clear, so I think the comment is no longer needed.

2 - 0001 - pg_subscription.c
```
@@ -542,12 +560,21 @@ HasSubscriptionTables(Oid subid)
+ * 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.
+ *
```

This function comment sounds a bit verbose and repetitive. Suggested revision:
```
* get_tables: if true, include tables in the returned list.
* get_sequences: if true, include sequences in the returned list.
* not_ready: if true, include only objects that have not reached the READY state;
* if false, include all objects of the requested type(s).
```

3 - 0001 - subscriptioncmds.c
```
+ * Currently, a replication slot is created for all subscriptions,
+ * including those for empty or sequence-only publications. While
+ * this is unnecessary, optimizing this behavior would require
+ * additional handling to ensure the apply worker operates
+ * smoothly without acquiring a slot on the publisher, thus adding
+ * complexity to the apply worker. Given that such subscriptions
+ * are infrequent, it doesn't seem to be worth doing anything
+ * about it.
```

Minor tweaks:
* "optimizing this behavior” -> “optimizing it”
* “doing anything about it” -> “addressing it"

4 - 0001 - subscriptioncmds.c
```
* 3) For ALTER SUBSCRIPTION ... REFRESH SEQUENCE statements with "copy_data =
* true" and "origin = none":
* - Warn the user that sequence data from another origin might have been
* copied.
```

“Warn the user” -> “Warn users"

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/