On Thursday, October 23, 2025 2:15 PM vignesh C <[email protected]> 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
