On Sat, Aug 12, 2023 at 5:01 AM Peter Smith <smithpb2...@gmail.com> wrote: > > On Fri, Aug 11, 2023 at 9:13 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Aug 11, 2023 at 3:41 PM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > On Fri, Aug 11, 2023 at 7:33 PM Amit Kapila <amit.kapil...@gmail.com> > > > wrote: > > > > > > > > On Thu, Aug 10, 2023 at 7:50 AM Peter Smith <smithpb2...@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > > > * If you do the above then there won't be a need to change the > > > > > > variable name is_parallel_apply_worker in logicalrep_worker_launch. > > > > > > > > > > > > > > > > Done. > > > > > > > > > > > > > I don't think the addition of two new macros isTablesyncWorker() and > > > > isLeaderApplyWorker() adds much value, so removed those and ran > > > > pgindent. I am planning to commit this patch early next week unless > > > > you or others have any comments. > > > > > > > > > > Thanks for considering this patch fit for pushing. > > > > > > Actually, I recently found 2 more overlooked places in the launcher.c > > > code which can benefit from using the isTablesyncWorker(w) macro that > > > was removed in patch v6-0001. > > > > > > > @@ -1301,7 +1301,7 @@ pg_stat_get_subscription(PG_FUNCTION_ARGS) > > worker_pid = worker.proc->pid; > > > > values[0] = ObjectIdGetDatum(worker.subid); > > - if (OidIsValid(worker.relid)) > > + if (isTablesyncWorker(&worker)) > > values[1] = ObjectIdGetDatum(worker.relid); > > > > I don't see this as a good fit for using isTablesyncWorker(). If we > > were returning worker_type then using it would be okay. > > Yeah, I also wasn't very sure about that one, except it seems > analogous to the existing code immediately below it, where you could > say the same thing: > if (isParallelApplyWorker(&worker)) > values[3] = Int32GetDatum(worker.leader_pid); >
Fair point. I think it is better to keep the code consistent. So, I'll merge your changes and push the patch early next week. -- With Regards, Amit Kapila.
