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...").
======
Kind Regards,
Peter Smith.
Fujitsu Australia
