Re: [PATCH] vacuumdb: Add --exclude-database option

From: Khoa Nguyen <kdnguyen9(dot)oss(at)gmail(dot)com>
To: Mohamed ALi <moali(dot)pg(at)gmail(dot)com>
Cc: Japin Li <japinli(at)hotmail(dot)com>, wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, Quan Zongliang <quanzongliang(at)yeah(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] vacuumdb: Add --exclude-database option
Date: 2026-06-12 23:47:56
Message-ID: CAONt3B3F8E__egL57JAmJd+MusgSUicS5NtT7ijGCe0y+0UxnQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 <moali(dot)pg(at)gmail(dot)com> 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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2026-06-13 00:25:05 Re: Commit Sequence Numbers and Visibility
Previous Message Sami Imseih 2026-06-12 23:21:42 Re: Improve pg_stat_statements scalability