Re: Logical Replication of sequences

On Mon, Jul 28, 2025 at 3:37 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:

> Thanks for the comments, the attached v20250728 version patch has the
> changes for the same.
>
Thanks for the patches, please find a few comments:

1)
WARNING: WITH clause parameters do not affect sequence synchronization

a)
How about:
WITH clause parameters are not applicable to sequence synchronization
or
WITH clause parameters are not applicable to sequence synchronization
and will be ignored.

b)
Should it be NOTICE OR WARNING? I feel NOTICE Is more appropriate as
it is more of an information than a warning since it has no negative
consequences.

2)
AlterSubscription_refresh(Subscription *sub, bool copy_data,
- List *validate_publications)
+ List *validate_publications, bool refresh_tables,
+ bool refresh_sequences, bool resync_all_sequences)
{

Do we need 3 new arguments? If we notice, 'refresh_sequences' is
always true in all cases. I feel only the last one should suffice.
IIUC, this is the state:

When resync_all_sequences is true:
it indicates it is 'REFRESH PUBLICATION SEQUENCES', that means we have
to refresh new sequences and resync all sequences.

When resync_all_sequences is false:
That means it is 'REFRESH PUBLICATION', we have to refresh new tables
and new sequences alone.

So if the caller pass only 'resync_all_sequences', we should be able
to drive the rest of the values internally.

3)
ALTER SUBSCRIPTION regress_testsub REFRESH PUBLICATION;
-ERROR: ALTER SUBSCRIPTION ... REFRESH cannot run inside a transaction block
+ERROR: ALTER SUBSCRIPTION ... REFRESH PUBLICATION cannot run inside
a transaction block

In the same script, we can test REFRESH PUBLICATION SEQUENCES also in
trancsation block.

4)
Commit message of patch004 says:

This patch introduce a new command to synchronize the sequences of
a subscription:
ALTER SUBSCRIPTION ... REFRESH PUBLICATION SEQUENCES

a)
introduce --> introduces

b)
We should also add:

This patch also changes the scope of
ALTER SUBSCRIPTION ... REFRESH PUBLICATION
This command now also considers sequences (newly added or dropped ones).

5)
+ * Reset the last_start_time of the sequencesync worker in the subscription's
+ * apply worker.

last_start_time-->last_seqsync_start_time

6)
alter_subscription.sgml has this:
<term><literal>refresh</literal> (<type>boolean</type>)</term>
<listitem>
<para>
When false, the command will not try to refresh table information.
<literal>REFRESH PUBLICATION</literal> should then be
executed separately.
The default is <literal>true</literal>.
</para>
</listitem>
</varlistentry>

Shouldn't we mention sequence too here:
When false, the command will not try to refresh table and sequence information.

thanks
Shveta