Обсуждение: pg_createsubscriber --dry-run logging concerns

Lately, I've been reviewing some pg_createsubscriber patches and have
been tricked by some of the logging.

The pg_createsubscriber has a '--dry-run' option/mode [1]

----------
--dry-run
Do everything except actually modifying the target directory.
----------

I've noticed that the logging in '--dry-run' mode is indistinguishable
from the logging of "normal" run, although functions like
create_publication(), drop_publication(), etc, are NOP in '--dry-run'
mode, because the actual command execution code is skipped. e.g.

if (!dry_run)
{
  res = PQexec(conn, str->data);
  ...
}

~~~

IMO, it's not good to fool people into thinking something has happened
when in fact nothing happened at all. I think the logging of this tool
should be much more explicit wrt when it is/isn't in dry-run mode.
Perhaps like this:

NORMAL
pg_log_info("creating publication \"%s\" in database \"%s\"", ...)

DRY-RUN
pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)

~~~

Thoughts?

======
[1] https://www.postgresql.org/docs/current/app-pgcreatesubscriber.html

Kind Regards,
Peter Smith.
Fujitsu Australia

Dear Peter,

> IMO, it's not good to fool people into thinking something has happened
> when in fact nothing happened at all. I think the logging of this tool
> should be much more explicit wrt when it is/isn't in dry-run mode.
> Perhaps like this:
> 
> NORMAL
> pg_log_info("creating publication \"%s\" in database \"%s\"", ...)
> 
> DRY-RUN
> pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)

Per my understanding, almost all the output must be adjusted based on the mode, right?
I feel it introduces a burden.
Can we solve the issue if we print additional message at the beginning if the
command runs with dry-run mode?

Best regards,
Hayato Kuroda
FUJITSU LIMITED 

On Wed, Oct 1, 2025 at 1:09 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Peter,
>
> > IMO, it's not good to fool people into thinking something has happened
> > when in fact nothing happened at all. I think the logging of this tool
> > should be much more explicit wrt when it is/isn't in dry-run mode.
> > Perhaps like this:
> >
> > NORMAL
> > pg_log_info("creating publication \"%s\" in database \"%s\"", ...)
> >
> > DRY-RUN
> > pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)
>
> Per my understanding, almost all the output must be adjusted based on the mode, right?
> I feel it introduces a burden.
> Can we solve the issue if we print additional message at the beginning if the
> command runs with dry-run mode?
>

Hi Kuroda-san,

Yes, that is one way. Something is better than nothing, at least...

I think that not *everything* in dry mode is fake; some of the logged
operations are real. So, it might be good if we can show fake ones
differently. e.g. It may not take much effort to introduce a wrapper
that inserts a prefix. Use this as a replacement for just a few
specific info logs.

(code below may not work; it's just for illustrative purposes)

#define pg_log_info_checkdry(...) do {\
  if (dry_run)\
    pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
  else;\
    pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
  } while (0);

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On 2025-Oct-01, Peter Smith wrote:

> (code below may not work; it's just for illustrative purposes)
> 
> #define pg_log_info_checkdry(...) do {\
>   if (dry_run)\
>     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
>   else;\
>     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
>   } while (0);

I like this kind of idea best.  However I think it might be better to do
it the other way around: have the normal pg_log_info() check dry_run,
and have a special one for the messages that are to be identical in
either mode.  I'm not sure how difficult this is to implement, though.

pg_subscriber is not the only program with a dry-run mode; it looks like
pg_archiveclean, pg_combinebackup, pg_resetwal, pg_rewind have one.  Is
it worth maybe doing something at the common/logging.c level rather than
specifically pg_createsubscriber?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
        Heiligenstädter Testament, L. v. Beethoven, 1802
        https://de.wikisource.org/wiki/Heiligenstädter_Testament

On Wed, Oct 1, 2025 at 8:37 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-01, Peter Smith wrote:
>
> > (code below may not work; it's just for illustrative purposes)
> >
> > #define pg_log_info_checkdry(...) do {\
> >   if (dry_run)\
> >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
> >   else;\
> >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
> >   } while (0);
>
> I like this kind of idea best.  However I think it might be better to do
> it the other way around: have the normal pg_log_info() check dry_run,
> and have a special one for the messages that are to be identical in
> either mode.  I'm not sure how difficult this is to implement, though.
>
> pg_subscriber is not the only program with a dry-run mode; it looks like
> pg_archiveclean, pg_combinebackup, pg_resetwal, pg_rewind have one.  Is
> it worth maybe doing something at the common/logging.c level rather than
> specifically pg_createsubscriber?
>

Hi Alvaro.

Thanks for the feedback and for pointing out the other tools that also
have a dr-run mode.

I did some quick back-of-the-envelope calculations to see what might
be involved.

======

For pg_create_subscriber, there are 38 info logs. Of those 38, I'd be
tempted to change only logs that say they are modifying/executing
something. There's ~15 of those:

pg_log_info("modifying system identifier of subscriber");
pg_log_info("running pg_resetwal on the subscriber");
pg_log_info("subscriber successfully changed the system identifier");
pg_log_info("use existing publication \"%s\" in database \"%s\"",
pg_log_info("create publication \"%s\" in database \"%s\"",
pg_log_info("create replication slot \"%s\" on publisher",
pg_log_info("dropping subscription \"%s\" in database \"%s\"",
pg_log_info("creating the replication slot \"%s\" in database \"%s\"",
pg_log_info("dropping the replication slot \"%s\" in database \"%s\"",
pg_log_info("creating publication \"%s\" in database \"%s\"",
pg_log_info("dropping publication \"%s\" in database \"%s\"",
pg_log_info("dropping all existing publications in database \"%s\"",
pg_log_info("creating subscription \"%s\" in database \"%s\"",
pg_log_info("setting the replication progress (node name \"%s\", LSN
%s) in database \"%s\"",
pg_log_info("enabling subscription \"%s\" in database \"%s\"",

======

For pg_archiveclean:

There only seems to be one logging for this dry run. Currently, it is
using debug logging like:

if (dry_run)
  pg_log_debug("would do XXX")
else
  pg_log_debug("doing XXX");

======

For pg_combinebackup:

This is the same as above; the differences for dry run logging are
already handled by having separate messages, and it uses debug logging
in many places with this pattern:

if (dry_run)
pg_log_debug("would do XXX")
else
    pg_log_debug("doing XXX");
======

For pg_resetwal:

This one is using a 'noupdate' flag instead of a 'dry_run' boolean.
And there doesn't seem to be any logging for this.

======

For pg_rewind:

Has 13 logs.  I'd be tempted to change maybe only a couple of these:

pg_rewind/pg_rewind.c: pg_log_info("creating backup label and updating
control file");
pg_rewind/pg_rewind.c: pg_log_info("executing \"%s\" for target server
to complete crash recovery",

======

I also found dry-run logs using the "would do XXX" pattern elsewhere,
in code like contrib/vacuumIo.c

//////

Summary

The idea to change the pg_log_info macro globally seems risky. There
are 400+ usages of this in the PG code, way beyond the scope of these
few tools that have a dry-run.

It seems that all the other --dry-run capable tools are using the pattern
if (dry_run)
pg_log_debug("would do XXX")
else
    pg_log_debug("doing XXX");

So, that makes pg_createsubscriber the odd man out. Instead of
introducing a new logging macro, perhaps it's better (for code
consistency) just to change pg_createsubscriber to use that same
logging pattern.

~~~

Kurdoa-san [1] was concerned it might be a big change burden to change
the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
that I'd be tempted to change in pg_createsubscriber.c; that's not
really such a burden.

OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
to indicate --dry-run might be useful. Even when run-time gives
different log messages, I think those other tools only show
differences when using DEBUG logging. Anybody not debugging might
otherwise have no idea that nothing actually happened.

~~

So, below is now my favoured solution:

1. Add an up-front info log to say when running in dry-run (add for
all tools that have --dry-run mode)

2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
consistent with all the other tools.
if (dry_run)
  pg_log_info("would do XXX")
else
  pg_log_info("doing XXX");

Thoughts?

======
[1]
https://www.postgresql.org/message-id/OSCPR01MB149668DD062C1457FFAA44A28F5E6A%40OSCPR01MB14966.jpnprd01.prod.outlook.com

Kind Regards,
Peter Smith.
Fujitsu Australia

On 2025-Oct-03, Peter Smith wrote:

> Summary
> 
> The idea to change the pg_log_info macro globally seems risky. There
> are 400+ usages of this in the PG code, way beyond the scope of these
> few tools that have a dry-run.

Ok.

> So, that makes pg_createsubscriber the odd man out. Instead of
> introducing a new logging macro, perhaps it's better (for code
> consistency) just to change pg_createsubscriber to use that same
> logging pattern.

Sure, let's go that way.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

On Thu, Oct 2, 2025, at 9:04 PM, Peter Smith wrote:
> Summary
>
> The idea to change the pg_log_info macro globally seems risky. There
> are 400+ usages of this in the PG code, way beyond the scope of these
> few tools that have a dry-run.
>
> It seems that all the other --dry-run capable tools are using the pattern
> if (dry_run)
> pg_log_debug("would do XXX")
> else
>     pg_log_debug("doing XXX");
>
> So, that makes pg_createsubscriber the odd man out. Instead of
> introducing a new logging macro, perhaps it's better (for code
> consistency) just to change pg_createsubscriber to use that same
> logging pattern.
>

What do you mean by "use the same logging pattern"? During development we
discussed if it is worth to double the number of messages to have accurate log
messages in dry run mode but decided it isn't.

I didn't understand all details of your proposal but I would like to say that
I'm against changing the 2 level log messages. Sometimes we just want a list of
the steps with the exact object names (hence, --verbose) instead of a bunch of
additional debug messages that exposes the implementation details (hence,
--verbose --verbose).

> Kurdoa-san [1] was concerned it might be a big change burden to change
> the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
> that I'd be tempted to change in pg_createsubscriber.c; that's not
> really such a burden.
>
> OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
> to indicate --dry-run might be useful. Even when run-time gives
> different log messages, I think those other tools only show
> differences when using DEBUG logging. Anybody not debugging might
> otherwise have no idea that nothing actually happened.
>

I agree that it seems confusing if you are not the one that wrote the command
line. Maybe it would be less confusing if we have a log message at the
beginning saying "pg_createsubscriber is in dry run mode".

> So, below is now my favoured solution:
>
> 1. Add an up-front info log to say when running in dry-run (add for
> all tools that have --dry-run mode)
>
> 2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
> consistent with all the other tools.
> if (dry_run)
>   pg_log_info("would do XXX")
> else
>   pg_log_info("doing XXX");
>

I particularly think a prefix increases the translation effort. As Alvaro said
if you want to propose a prefix feature, it should cover the other tools that
use the logging module too.

Since you are improving messages, I suggest 2 changes:

pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
pg_createsubscriber: create replication slot "pg_createsubscriber_16384_710455e1" on publisher

to

pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"

because it is duplicated.

Don't refer to the database name for physical replication slot

pg_createsubscriber: dropping the replication slot "primaryslot" in database "bench1"

Maybe replace "in database foo" with "on primary".


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

Hi Euler,

On Tue, Oct 7, 2025 at 7:06 AM Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, Oct 2, 2025, at 9:04 PM, Peter Smith wrote:
> > Summary
> >
> > The idea to change the pg_log_info macro globally seems risky. There
> > are 400+ usages of this in the PG code, way beyond the scope of these
> > few tools that have a dry-run.
> >
> > It seems that all the other --dry-run capable tools are using the pattern
> > if (dry_run)
> > pg_log_debug("would do XXX")
> > else
> >     pg_log_debug("doing XXX");
> >
> > So, that makes pg_createsubscriber the odd man out. Instead of
> > introducing a new logging macro, perhaps it's better (for code
> > consistency) just to change pg_createsubscriber to use that same
> > logging pattern.
> >
>
> What do you mean by "use the same logging pattern"? During development we
> discussed if it is worth to double the number of messages to have accurate log
> messages in dry run mode but decided it isn't.
>

Yes, one part I proposed is to do exactly that. i.e. double-up on some
messages (about a dozen of them), because that is what the other tools
that have '--dry-run' mode are doing. They have messages like:
"creating xxx" and related one for --dry-run that says "would create xxx"

So I was proposing to do the same, consistent code pattern with the other tools.

> I didn't understand all details of your proposal but I would like to say that
> I'm against changing the 2 level log messages. Sometimes we just want a list of
> the steps with the exact object names (hence, --verbose) instead of a bunch of
> additional debug messages that exposes the implementation details (hence,
> --verbose --verbose).

Yeah, I understand that the other places (like pg_combinebackup.c) are
using pg_log_debug instead of pg_log_info, so perhaps your point is
that although it was OK to do this in debug, you think doing the same
for pg_log_info is a bridge too far?

I am not wedded to doing this double-messaging... if people feel just
the one-time logging at the beginning is enough, then that is OK by
me.

>
> > Kurdoa-san [1] was concerned it might be a big change burden to change
> > the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
> > that I'd be tempted to change in pg_createsubscriber.c; that's not
> > really such a burden.
> >
> > OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
> > to indicate --dry-run might be useful. Even when run-time gives
> > different log messages, I think those other tools only show
> > differences when using DEBUG logging. Anybody not debugging might
> > otherwise have no idea that nothing actually happened.
> >
>
> I agree that it seems confusing if you are not the one that wrote the command
> line. Maybe it would be less confusing if we have a log message at the
> beginning saying "pg_createsubscriber is in dry run mode".
>

Yes. So, please see the attached patch that implements this. And for
consistency, the change is repeated to all other tools that use
--dry-run mode.

> > So, below is now my favoured solution:
> >
> > 1. Add an up-front info log to say when running in dry-run (add for
> > all tools that have --dry-run mode)
> >
> > 2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
> > consistent with all the other tools.
> > if (dry_run)
> >   pg_log_info("would do XXX")
> > else
> >   pg_log_info("doing XXX");
> >
>
> I particularly think a prefix increases the translation effort. As Alvaro said
> if you want to propose a prefix feature, it should cover the other tools that
> use the logging module too.
>

There are no plans to have a prefix feature. That was an initial
thought bubble, but after I saw how all the other tools that have
--dry-run just have pairs of logging, I dropped the prefix idea.

> Since you are improving messages, I suggest 2 changes:
>
> pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
> pg_createsubscriber: create replication slot "pg_createsubscriber_16384_710455e1" on publisher
>
> to
>
> pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
>
> because it is duplicated.
>
> Don't refer to the database name for physical replication slot
>
> pg_createsubscriber: dropping the replication slot "primaryslot" in database "bench1"
>
> Maybe replace "in database foo" with "on primary".
>

Actually. I had already created another thread/patch about that
duplicate logging issue [1].

As for all the other suggestions, I'd rather keep this thread scope
focused on the '--dry-run' logging issue, and not conflate it with all
those other logging problems, which probably all deserve their own
threads/patches.

======
[1] https://www.postgresql.org/message-id/CAHut+Pv7qDvLbDgc9PQGhULT3rPXTxdu_=w+iW-kMs+zPADR+w@mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

On 2025-Oct-08, Peter Smith wrote:

> I am not wedded to doing this double-messaging... if people feel just
> the one-time logging at the beginning is enough, then that is OK by
> me.

Some other tools (not Postgres ones) do that and it always makes me
nervous, because I can never be sure which parts were actually done and
which parts are only dry-run trials.  I'd rather stay away from that
approach.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"¿Cómo puedes confiar en algo que pagas y que no ves,
y no confiar en algo que te dan y te lo muestran?" (Germán Poo)

On Sat, Oct 4, 2025 at 3:50 AM Neeraj Shah <n4j@liamg.xyz> wrote:
>
> Hello Peter,
>
> Your suggestion makes sense to me, if it's fine with you then I can submit a patch for this change :).
>

Thanks, but I already had some local patches ready to go.... I was
just waiting for more feedback before posting them.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Wed, Oct 8, 2025 at 9:04 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-08, Peter Smith wrote:
>
> > I am not wedded to doing this double-messaging... if people feel just
> > the one-time logging at the beginning is enough, then that is OK by
> > me.
>
> Some other tools (not Postgres ones) do that and it always makes me
> nervous, because I can never be sure which parts were actually done and
> which parts are only dry-run trials.  I'd rather stay away from that
> approach.
>

OK. Here are v2 patches to implement both ways. You can pick one or both.

0001 - a beginning log to say if the tool is in dry-run mode
0002 - alternate pg_log_info messages for pg_createsubscriber, when in
dry-run mode

~

Note: there might be a clash with another thread [1] if that gets
committed soon; I will deal with any rebase if/when it is needed.

======
[1]
https://www.postgresql.org/message-id/flat/CAHut%2BPv7qDvLbDgc9PQGhULT3rPXTxdu_%3Dw%2BiW-kMs%2BzPADR%2Bw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

On Thu, Oct 9, 2025 at 4:38 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> I think is patch is helpful. A few comments:
>
> On Oct 9, 2025, at 08:55, Peter Smith <smithpb2250@gmail.com> wrote:
>
>
<v2-0001-log-to-say-command-is-executing-in-dry-run-mode.patch><v2-0002-add-different-dry-run-logging-for-pg_createsubscr.patch>
>
>
>
> 1 - 0001
> ```
> + if (dryrun)
> + {
> + pg_log_info("-----------------------------------------------------");
> + pg_log_info("pg_archivecleanup is executing in '--dry-run' mode.");
> + pg_log_info("No files will be removed.");
> + pg_log_info("-----------------------------------------------------");
> + }
> ```
>
> Putting the program name in log message feels redundant, because pg_log_info() may already prefixes logs with program
name.But I like the separator lines that make it stand out visually in logs. So this log can be simplified as: 
>
> ```

Fair point. I removed the tool name from the message.

> 2 - 0002
> ```
> - if (!dry_run)
> + if (dry_run)
> + pg_log_info("in dry-run mode, otherwise system identifier would be %" PRIu64 " on subscriber",
> + cf->system_identifier);
> ```
>
> I think this log message can be simplified as:
>
> ```
> pg_log_info("dry-run: system identifier would be %" PRIu64 " on subscriber",
>             cf->system_identifier);
> ```
>
> As a general comment, I think “in dry-run mode” is a little long-winded, we can just put “dry-run:”, and “otherwise”
seemsnot needed because “dry-run” has clearly indicated nothing would actually happen. This comment applies to all
changesin pg_createsubscriber.c of 0002. 
>

OK. Updated as suggested.

~~

The anticipated rebase was necessary, too.

Please see the v3 patches.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On 2025-Oct-09, Peter Smith wrote:

> Please see the v3 patches.

Okay, I have pushed 0002 to 18 and master.  I wanted to backpatch to 17
but there are too many conflicts there.

I'm not opposed to 0001 (to master only), but I think the lines of
dashes are a little excessively noisy.  Are there other opinions on
that?  Note that vacuumlo also has it, with no dashes.

I'll mark the commitfest item for version 19.

While looking this over, I noticed that we define USEC_PER_SEC, but why?
We already have USECS_PER_SEC, so let's use that.  And WaitPMResult
seems an overgrown boolean, so how about we remove it?  Patch for these
things attached.  Thoughts?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Thou shalt study thy libraries and strive not to reinvent them without
cause, that thy code may be short and readable and thy days pleasant
and productive. (7th Commandment for C Programmers)

On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-09, Peter Smith wrote:
>
> > Please see the v3 patches.
>
> Okay, I have pushed 0002 to 18 and master.  I wanted to backpatch to 17
> but there are too many conflicts there.
>

Thanks for pushing!

> I'm not opposed to 0001 (to master only), but I think the lines of
> dashes are a little excessively noisy.  Are there other opinions on
> that?  Note that vacuumlo also has it, with no dashes.

It was intentionally "noisy" with dashes to make it impossible to
accidentally overlook dry-run mode in the logs. But if that's contrary
to the way Postgres does things I am fine if you want to remove the
dashes.

>
> I'll mark the commitfest item for version 19.
>

> While looking this over, I noticed that we define USEC_PER_SEC, but why?
> We already have USECS_PER_SEC, so let's use that.  And WaitPMResult
> seems an overgrown boolean, so how about we remove it?  Patch for these
> things attached.  Thoughts?
>

I took a quick look at your follow-up patches and below are some comments:

//////////
Patch 0001 - USECS_PER_SEC
//////////

+1 to use USECS_PER_SEC from timestamps.h

1.
-#define USEC_PER_SEC 1000000
-
-#define WAITS_PER_SEC 10 /* should divide USEC_PER_SEC evenly */
+#define WAITS_PER_SEC 10 /* should divide USECS_PER_SEC evenly */

 static bool do_wait = true;
 static int wait_seconds = DEFAULT_WAIT;
@@ -594,6 +593,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
 {
  int i;

+ static_assert(USECS_PER_SEC % WAITS_PER_SEC == 0);

Maybe that static assert might be better placed adjacent to the
#define to enforce that comment about "divide evenly".

SUGGESTION
#define WAITS_PER_SEC 10

/* WAITS_PER_SEC should divide USECS_PER_SEC evenly */
StatiAssertDecl(USECS_PER_SEC % WAITS_PER_SEC == 0);

//////////
Patch 0002 -- boolean
//////////

LGTM.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On 2025-Nov-04, Peter Smith wrote:

> On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:

> > I'm not opposed to 0001 (to master only), but I think the lines of
> > dashes are a little excessively noisy.  Are there other opinions on
> > that?  Note that vacuumlo also has it, with no dashes.
> 
> It was intentionally "noisy" with dashes to make it impossible to
> accidentally overlook dry-run mode in the logs. But if that's contrary
> to the way Postgres does things I am fine if you want to remove the
> dashes.

Yeah, I don't know if it's The Postgres Way or just accidental, but I'd
rather stay away from that kind of noise.  Also, from a i18n point of
view, the lines of dashes (as implemented here) would get in the message
catalogs, which nobody will appreciate.  Please do make the messages a
single string rather than two separate strings though, perhaps with an
embedded newline if more than one line of output is needed.

> I took a quick look at your follow-up patches and below are some comments:

Thanks for looking at them.  I pushed 0002; I'm waiting on the CI run
https://cirrus-ci.com/build/5618025065349120
to push 0001.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/