On Thu, Nov 6, 2025 at 7:18 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Wed, Nov 5, 2025 at 3:42 PM Shubham Khanna
> <khannashubham1197@gmail.com> wrote:
> >
> > On Mon, Nov 3, 2025 at 12:55 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Hi Shubham.
> > >
> > > A comment about the v17-0001.
> > >
> > > ======
> > > 1.
> > > + if (check_publication_exists(conn, dbinfo[i].pubname, dbinfo[i].dbname))
> > > + {
> > > + /* Reuse existing publication on publisher. */
> > > + pg_log_info("dry-run: would use existing publication \"%s\" in
> > > database \"%s\"",
> > > + dbinfo[i].pubname, dbinfo[i].dbname);
> > > + dbinfo[i].made_publication = false;
> > > + }
> > >
> > > Is that correct? Won't this code now unconditionally log with the
> > > "dry-run:" prefix, even when the tool is *not* doing a dry-run?
> > >
> > > I thought code would be something like:
> > >
> > > SUGGESTION #1 (if/else)
> > > /* Reuse existing publication on publisher. */
> > > if (dry_run)
> > > pg_log_info("dry-run: would use existing publication ...);
> > > else
> > > pg_log_info("use existing publication ...);
> > >
> > > ~~~
> > >
> > > OTOH, (since here is just an info message with no destructive
> > > operation) perhaps it would be harmless also to keep the original log
> > > message for both dry-run and normal mode.
> > >
> > > SUGGESTION #2 (do nothing)
> > > pg_log_info("use existing publication ...);
> > >
> > > ======
> >
> > Hi Peter,
> >
> > Thank you for your review and suggestions.
> > I agree with your reasoning regarding the logging behavior. I will
> > proceed with Suggestion #2 and retain the existing `pg_log_info("use
> > existing publication ...");` message for both dry-run and normal
> > modes. This message is purely informational and does not perform any
> > destructive action, making it suitable for both scenarios.
> > The attached patch includes these changes.
> >
>
> OK, but you did not do quite what you said you did.
>
> Notice that v18 has modified the message to "would use existing
> publication...", instead of just leaving it how it was in v16 ("use
> existing publication...").
>
> ======
Fixed.
The attached patch includes these changes.
Thanks and regards,
Shubham Khanna.
