Re: vacuumdb: add --dry-run

Hi Nathan,

I just reviewed v5, and overall looks very good patch quality. Just a few nit comments on 0001 and 0003.

> On Dec 5, 2025, at 05:52, Nathan Bossart <nathandbossart@gmail.com> wrote:
> 
> 
> -- 
> nathan
> <v5-0001-Move-some-vacuumdb-options-to-vacopts-struct.patch><v5-0002-Add-ParallelSlotSetIdle.patch><v5-0003-Add-dry-run-to-vacuumdb.patch>


1 - 0001
```
    /* initialize options */
memset(&vacopts, 0, sizeof(vacopts));
vacopts.objfilter = 0; /* no filter */
vacopts.parallel_workers = -1;
vacopts.buffer_usage_limit = NULL;
vacopts.no_index_cleanup = false;
vacopts.force_index_cleanup = false;
vacopts.do_truncate = true;
vacopts.process_main = true;
vacopts.process_toast = true;
```

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.

2 - 0003
```
+	if (echo || dry_run)
+		printf("%s%s\n", sql, dry_run ? "  -- not executed" : "");
```

There are two white-spaces before “--“, I think one is enough, In the other place of 0003, you just one white-space before “--“.

3 - 0003
```
@@ -1001,15 +1009,19 @@ prepare_vacuum_command(PGconn *conn, PQExpBuffer sql,
  * Any errors during command execution are reported to stderr.
  */
 static void
-run_vacuum_command(PGconn *conn, const char *sql, bool echo,
-				   const char *table)
+run_vacuum_command(ParallelSlot *free_slot, const char *sql,
+				   bool echo, bool dry_run, const char *table)
 {
```

Here are two comments:

* 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.

4 - 0003
```
+	if (vacopts.dry_run)
+		pg_log_info("Executing in dry-run mode.”);
```

Feels like “Running” is better than “Executing”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/