From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Gilles Darold <gilles(at)migops(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Robert Treat <rob(at)xzilla(dot)net>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [Proposal] vacuumdb --schema only |
Date: | 2022-04-26 04:46:55 |
Message-ID: | 20220426044655.GA2998678@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Apr 26, 2022 at 11:36:02AM +0900, Michael Paquier wrote:
> The refactoring logic to build the queries is clear to follow. I have
> a few comments about the shape of the patch, though.
Thanks for taking a look!
> case 'a':
> - alldb = true;
> + check_objfilter(OBJFILTER_ALL_DBS);
> break;
> The cross-option checks are usually done after all the options
> switches are check. Why does this need to be different? It does not
> strike me as a huge problem to do one filter check at the end.
Makes sense. I fixed this in v13.
> +void
> +check_objfilter(VacObjFilter curr_option)
> +{
> + objfilter |= curr_option;
> +
> + if ((objfilter & OBJFILTER_ALL_DBS) &&
> + (objfilter & OBJFILTER_DATABASE))
> + pg_fatal("cannot vacuum all databases and a specific one at the same time");
> The addition of more OBJFILTER_* (unlikely going to happen, but who
> knows) would make it hard to know which option should not interact
> with each other. Wouldn't it be better to use a kind of compatibility
> table for that? As one OBJFILTER_* matches with one option, you could
> simplify the number of strings in need of translation by switching to
> an error message like "cannot use options %s and %s together", or
> something like that?
I think this might actually make things more complicated. In addition to
the compatibility table, we'd need to define the strings to use in the
error message somewhere. I can give this a try if you feel strongly about
it.
> +$node->command_fails(
> + [ 'vacuumdb', '-a', '-d', 'postgres' ],
> + 'cannot use options -a and -d at the same time');
> This set of tests had better use command_fails_like() to make sure
> that the correct error patterns from check_objfilter() show up?
Yes. I did this in v13.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v13-0001-Add-schema-and-exclude-schema-options-to-vacuumd.patch | text/x-diff | 17.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Julien Rouhaud | 2022-04-26 04:54:35 | Re: BUG #17448: In Windows 10, version 1703 and later, huge_pages doesn't work. |
Previous Message | Michael Paquier | 2022-04-26 02:53:54 | Re: [PATCH] Teach pg_waldump to extract FPIs from the WAL |