|From:||Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>|
|To:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>|
|Subject:||Re: pg_dumpall --exclude-database option|
|Views:||Raw Message | Whole Thread | Download mbox|
On 08/03/2018 05:08 PM, Fabien COELHO wrote:
>> Among other use cases, this is useful where a database name is
>> visible but the database is not dumpable by the user. Examples of
>> this occur in some managed Postgres services.
> This looks like a reasonable feature.
Thanks for the review.
>> I will add this to the September CF.
> My 0.02€:
> Patch applies cleanly, compiles, and works for me.
> A question: would it makes sense to have a symmetrical
> --include-database=PATTERN option as well?
I don't think so. If you only want a few databases, just use pg_dump.
The premise of pg_dumpall is that you want all of them and this switch
provides for exceptions to that.
> Somehow the option does not make much sense when under -g/-r/-t...
> maybe it should complain, like it does when the others are used together?
Added an error check.
> ISTM that it would have been better to issue just one query with an OR
> list, but that would require to extend "processSQLNamePattern" a
> little bit. Not sure whether it is worth it.
I don't think it is. This uses the same pattern that is used in
pg_dump.c for similar switches.
> Function "database_excluded": I'd suggest to consider reusing the
> "simple_string_list_member" function instead of reimplementing it in a
> special case.
> XML doc: "--exclude-database=dbname", ISTM that
> "--exclude-database=pattern" would be closer to what it is? "Multiple
> database can be matched by writing multiple switches". Sure, but it
> can also be done with a pattern. The documentation seems to assume
> that the argument is one database name, and then changes this
> afterwards. I'd suggest to start by saying that a pattern like psql is
> expected, and then proceed to simply tell that the option can be
> repeated, instead of implying that it is a dbname and then telling
> that it is a pattern.
> The simple list is not freed. Ok, it seems to be part of the design of
> the data structure.
I don't see much point in freeing it.
revised patch attached.
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
|Next Message||Andres Freund||2018-10-08 16:30:49||Re: out-of-order XID insertion in KnownAssignedXids|
|Previous Message||Andres Freund||2018-10-08 15:59:04||Re: PostgreSQL 12, JIT defaults|