Re: pg_dumpall --exclude-database option

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_dumpall --exclude-database option
Date: 2018-10-08 16:09:29
Message-ID: f287a861-b742-9595-2cfb-67290822987e@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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.

done.

>
> 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.

docco revised.

>
> 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.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
pg_dumpall-exclude-v3.patch text/x-patch 7.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
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