This patch can be very useful for clusters with a large number of databases.
On Thu, Jun 11, 2026 at 12:35 AM Mohamed ALi <[email protected]> wrote: > > Hi Japin, > > Thanks for the review. > > On Tue, Jun 10, 2026 at 09:39:16 AM, Japin Li wrote: > > +$node->command_fails_like( > > + [ 'vacuumdb', '-d' => 'postgres', '--exclude-database' => > > 'regression_excl_test' ], > > + qr/cannot use --exclude-database without --all option/, > > + 'cannot use --exclude-database with -d'); > > > > The test is a bit confusing to me. It does not state that > > --exclude-database cannot be used with the -d option. > > Good catch. The issue was that "vacuumdb -d postgres -D test_db1" > produced "cannot use --exclude-database without --all option", which > is misleading — if the user then adds --all, they hit a second error > ("cannot vacuum all databases and a specific one at the same time"). > > Before (v2): > > $ vacuumdb -D test_db1 > vacuumdb: error: cannot use --exclude-database without --all option > $ vacuumdb -d postgres -D test_db1 > vacuumdb: error: cannot use --exclude-database without --all option > $ vacuumdb -d postgres -D test_db1 --all > vacuumdb: error: cannot vacuum all databases and a specific one at > the same time > > After (v3): > > $ vacuumdb -D test_db1 > vacuumdb: error: cannot use the "exclude-database" option without > the "all" option > $ vacuumdb -d postgres -D test_db1 > vacuumdb: error: cannot use the "exclude-database" option with the > "dbname" option > $ vacuumdb -d postgres -D test_db1 --all > vacuumdb: error: cannot vacuum all databases and a specific one at > the same time > > v3 adds a separate check that fires first when both -d and > --exclude-database are used together, giving a clear error instead > of bouncing the user between two messages. > > Changes in v3: > - Added a check for OBJFILTER_DATABASE_EXCLUDE + OBJFILTER_DATABASE > that fires before the "without all" check, giving a clear error > when both options are specified together. > - Both error messages now use the pg_fatal("%s") format consistent > with other option-conflict errors in vacuumdb.c. > - Updated TAP test to match the new error messages. I've installed the v2 and tried out the exclusion option. Everything works as expected. When I reviewed the code, what jumped out and concerned me the most was the dynamic query built by injecting the excluded db literals into the NOT IN list of the query. Looking at this block of code closely, I saw that the author used appendStringLiteralConn which uses PQescapeStringConn. This is the commonly used API for escaping strings in client tools so we are good. Having the ability to exclude means that we have to deal with numdbs being 0. This is well gated with "for (int i = 0; i < numdbs; i++)". When executed with --analyze-in-stages --missing-stats-only, palloc0(0) -> pg_malloc_internal(0)->malloc(1) and pg_free is also safe with 1 byte. I walked through the code path of "if (vacopts->mode == MODE_ANALYZE_IN_STAGES)" in gdb and found no issue for numdbs=0. Overall I think the patch looks good. I'll review v3 soon.
