Date: Fri, 5 Dec 2025 14:08:06 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Corey Huinker <corey.huinker@gmail.com>,
	Chao Li <li.evan.chao@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: vacuumdb: add --dry-run
Message-ID: <aTM7pgNQesAp79Pn@nathan>
References: <3DB353E5-944D-4E18-9DA2-4151F44AA59B@gmail.com>
 <CADkLM=dgpe7wgjLhnpTycPRzUn6_U9Tk8qNAxHALi4-JqgPp=g@mail.gmail.com>
 <aR5IMz4cngn8hkXI@nathan>
 <CADkLM=dwBjL5fK-Dq_2qhAuHsHPab_Kt+R7KZTA9w_X0K6bENg@mail.gmail.com>
 <CADkLM=d+XROe3JgM5gZno0JGEgxnDDCYsEHgQjg5x7d0OQp5vA@mail.gmail.com>
 <aR-MQgYi_FtSsy66@nathan>
 <CADkLM=ejhuvk9TOmYe09kaWdUoCzPci1aYa9+ADOwzP22e1XeA@mail.gmail.com>
 <aR-TLculrOb2hbRy@nathan>
 <aTICg5LV0Nl6khYj@nathan>
 <CALdSSPjEO-DoHoPyrV9RV8HOU9LXPyak-jJ=mxGXGU4ZA9QB8A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CALdSSPjEO-DoHoPyrV9RV8HOU9LXPyak-jJ=mxGXGU4ZA9QB8A@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aTM7pgNQesAp79Pn%40nathan>
Precedence: bulk

On Sat, Dec 06, 2025 at 12:56:22AM +0500, Kirill Reshke wrote:
> Hi!

Thanks for reviewing.

>> +     <varlistentry>
>> +      <term><option>--dry-run</option></term>
>> +      <listitem>
>> +       <para>
>> +        Print, but do not execute, the vacuum and analyze commands that would
>> +        have been sent to the server (performs a dry run).
>> +       </para>
>> +      </listitem>
>> +     </varlistentry>
> 
> I compared this smgl section to analogous sections of server utilities
> (pg_dump, pg_resetwal, pg_rewind), none of them mentions "dry run" in
> description of what "--dry run" is. So, I think my feeling is that
> this `(performs a dry run).` is unneeded is correct.

I borrowed this from pg_archivecleanup's documentation.  But to your point,
there doesn't seem to be a tremendous amount of consistency in the dry-run
options for various utilities.

>>  - printf(_("%s: vacuuming database \"%s\"\n"),
>> -   progname, PQdb(conn));
>> + printf(_("%s: vacuuming database \"%s\"%s\n"),
>> +   progname, PQdb(conn),
>> +   vacopts->dry_run ? " (dry-run)" : "");
> 
> I am also not sure we need this change. Look:
> 
> ```
> reshke@yezzey-cbdb-bench:~/pg$ ./bin/bin/vacuumdb --dry-run
> vacuumdb: Executing in dry-run mode.
> vacuumdb: vacuuming database "reshke" (dry-run)
> ```
> 
> We have two lines which say the same. Well, maybe there is value in
> this change, if we are vacuuming multiple databases, but given that
> --dry-run produces a lot of
> `VACUUM ...  -- not executed` output, I think It will be obvious that
> this vacuumdb run does not modify the system. WDYT?

I guess we could probably remove the top-level "Executing in dry-run mode"
message, provided we say the same thing in the per-database message.
However, the latter can be turned off with --quiet.  Maybe we should
consider disallowing --quiet and --dry-run.

Overall, I can't claim to have super principled arguments about where I've
added these dry-run messages.  I kind-of just sprinkled them around.

-- 
nathan