On Friday, February 27, 2026 7:07 PM Amit Kapila <[email protected]> wrote: > Few comments: > ============= > 1. > + oldctx = MemoryContextSwitchTo(SequenceSyncContext); > > - initStringInfo(&app_name); > - appendStringInfo(&app_name, "pg_%u_sequence_sync_" UINT64_FORMAT, > - MySubscription->oid, GetSystemIdentifier()); > + /* Process sequences */ > + sequence_copied = copy_sequences(conn, seqinfos); > > - /* > - * Establish the connection to the publisher for sequence synchronization. > - */ > - LogRepWorkerWalRcvConn = > - walrcv_connect(MySubscription->conninfo, true, true, > - must_use_password, > - app_name.data, &err); > - if (LogRepWorkerWalRcvConn == NULL) > - ereport(ERROR, > - errcode(ERRCODE_CONNECTION_FAILURE), > - errmsg("sequencesync worker for subscription \"%s\" could not connect to > the publisher: %s", > - MySubscription->name, err)); > - > - pfree(app_name.data); > - > - copy_sequences(LogRepWorkerWalRcvConn); > + MemoryContextSwitchTo(oldctx); > > It is better to switch to SequenceSyncContext at the caller of > LogicalRepSyncSequences similar to what we are doing for > ApplyMessageContext.
Agreed. I changed as suggested. > > 2. > @@ -4221,6 +4221,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received) > ProcessConfigFile(PGC_SIGHUP); > } > > + > if (rc & WL_TIMEOUT) > > Spurious line addition. > > 3. Apart from above, the attached patch contains comments and cosmetic > changes. Thanks for the patch, I have merged them into 0001. Here is the V8 patch set addressing the previous review comments: - For 0001, I noticed that the GetSequence() function added in the patch fetches the local sequence value without any privilege check. This allows the worker to read sequence data even without proper SELECT privilege, which seems unsafe. I've added a SELECT privilege check before fetching the sequence value. Additionally, I've updated several comments, made cosmetic changes, commit message update, and run pgindent on all patches. - 0002 includes the changes to synchronize sequences directly in the REFRESH SEQUENCES command Best Regards, Hou zj
v8-0001-Support-automatic-sequence-replication.patch
Description: v8-0001-Support-automatic-sequence-replication.patch
v8-0002-Synchronize-sequences-directly-in-REFRESH-SEQUENC.patch
Description: v8-0002-Synchronize-sequences-directly-in-REFRESH-SEQUENC.patch
