RE: Logical Replication of sequences

On Thursday, October 23, 2025 2:15 PM vignesh C <vignesh21@gmail.com> wrote:
> The attached patch has the changes for the same.
> I have also addressed the other comments: a) Shveta's comments at [1]
> b) Peter's comments at [2] & [3] c) Shveta's 2nd patch comments at [4] and d)
> Chao's comment#12 from [5] which was pending.

Thanks for updating the patch, I have a few comments for 0002.

1.

+        hash_seq_init(&hash_seq, sequences_to_copy);
+        while ((seq_entry = hash_seq_search(&hash_seq)) != NULL)
+        {
+            pfree(seq_entry->seqname);
+            pfree(seq_entry->nspname);
+        }
+    }
+
+    hash_destroy(sequences_to_copy);

I personally feel these memory free calls are unnecessary since the sync worker
will stop soon.


2.
-FinishSyncWorker(void)
+FinishSyncWorker(LogicalRepWorkerType wtype)

Can we directly access MyLogicalRepWorker->type instead of adding
one func parameter ?


3.

                         "ORDER BY s.schname, s.seqname\n",

Just to confirm, is this "ORDER BY" necessary for correctness ?


4.

        elog(LOG, "skip synchronization of sequence \"%s.%s\" because it has been dropped concurrently",
             nspname, seqname);

Shall we use ereport here ?

Best Regards,
Hou zj