Re: vacuumdb: add --dry-run

On Sat, Dec 06, 2025 at 07:48:22AM +0800, Chao Li wrote:
> I just reviewed v5, and overall looks very good patch quality. Just a few
> nit comments on 0001 and 0003.

I've attached an updated patch set.  After giving the "dry-run" messages
another look, I think we should just print a note at the very beginning of
vacuumdb execution and leave it at that.  The per-database messages weren't
translator friendly and IMHO didn't add much, and the "-- not executed"
comments were noisy and didn't reflect the commands that would've been sent
to the server.

> Now echo and print are moved into vacopts and their default values are
> false. Here, memset() have properly initialized their values. But this
> piece of code still explicitly set boolean values to vacopts fields. So,
> to make it consistent, I feel we can also add explicit assignments to
> echo and print here, or remove those “false” assignments. This is not a
> correctness issue, just to keep in a consistent style.

We are already pretty inconsistent about this.  If anything, I think we
should do the opposite, i.e., remove any unnecessary initializations to
0/false/NULL.  The memset() makes those redundant and should suffice in
most cases.

> * As run_vacuum_command() takes both echo and dry_run, and both of them
> are defined in vcaopts, why not change this function to take a const
> vcaopts * instead of two bools?
> 
> * The function comment needs to be updated. Now it won’t always send a
> command to server, with “dry_run”, it behaves differently.

Done.

> ```
> +	if (vacopts.dry_run)
> +		pg_log_info("Executing in dry-run mode.”);
> ```
> 
> Feels like “Running” is better than “Executing”.

Done.

-- 
nathan