Re: [Proposal] vacuumdb --schema only

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>, 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-07-31 20:50:31
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2022-04-26 Tu 00:46, Nathan Bossart wrote:
> 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.




Andrew Dunstan

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2022-07-31 21:19:34 Re: pg_auth_members.grantor is bunk
Previous Message Andres Freund 2022-07-31 20:31:58 Re: Cirrus CI (Windows help wanted)