Re: [PATCH] Add --create-only option to pg_dump/pg_dumpall - Mailing list pgsql-hackers
| From | Nitin Jadhav |
|---|---|
| Subject | Re: [PATCH] Add --create-only option to pg_dump/pg_dumpall |
| Date | |
| Msg-id | CAMm1aWY-WKPZ7qvGMVaoieObTYKKjhZp5uihFX+O89X3jrGAnw@mail.gmail.com Whole thread Raw |
| In response to | Re: [PATCH] Add --create-only option to pg_dump/pg_dumpall (Michael Banck <michael.banck@credativ.de>) |
| Responses |
Re: [PATCH] Add --create-only option to pg_dump/pg_dumpall
(Daniel Gustafsson <daniel@yesql.se>)
|
| List | pgsql-hackers |
Hi,
I have reviewed and tested the patch. Following are a few comments.
1.
The main objective of this patch is to get the dump which consists of SQLs related to
CREATEDB only. I have tested the patch and it generates a proper dump file. But my
concern is, it should execute the code which is necessary. But I see that the code
is preparing some data which we may not dump. So I feel we should avoid executing
such flows. Please correct me if I am wrong.
2.
> > I also agree that the option name is a little misleading to the user
> > so I would suggest instead of using "create-only", you can say
> > something maybe like "createdb-only" because this option only applies
> > to CREATE DATABASE related commands, not CREATE TABLE or other
> > objects. In the help menu, you can then elaborate more that this
> > option "dump only the commands related to create database like ALTER,
> > GRANT..etc"
>
> Well I have to say I agree with Peter that the option name I came up
> with is pretty confusing, not sure createdb-only is much better as it
> also includes GRANTs etc.
> > so I would suggest instead of using "create-only", you can say
> > something maybe like "createdb-only" because this option only applies
> > to CREATE DATABASE related commands, not CREATE TABLE or other
> > objects. In the help menu, you can then elaborate more that this
> > option "dump only the commands related to create database like ALTER,
> > GRANT..etc"
>
> Well I have to say I agree with Peter that the option name I came up
> with is pretty confusing, not sure createdb-only is much better as it
> also includes GRANTs etc.
I agree with Cary that we should name this as 'createdb-only' and provide a brief
description in help.
3.
if (!plainText)
dopt.outputCreateDB = 1;
+ if (dopt.outputCreateDBOnly)
+ dopt.outputCreateDB = 1;
+
'dopt.outputCreateDBOnly' if block can be merged with '!plainText' if block.
4.
static int binary_upgrade = 0;
static int column_inserts = 0;
+static int create_only = 0;
static int disable_dollar_quoting = 0;
The variable 'create_only' should be changed to 'createdb_only' to match with
similar variable used in pg_dump.c.
Thanks and Regards,
Nitin Jadhav
On Tue, Mar 30, 2021 at 9:32 PM Michael Banck <michael.banck@credativ.de> wrote:
Hi,
Am Montag, den 29.03.2021, 17:59 +0000 schrieb Cary Huang:
> I have tried the patch and the new option is able to control the
> contents of pg_dump outputs to include only create db related
> commands.
Thanks for testing!
> I also agree that the option name is a little misleading to the user
> so I would suggest instead of using "create-only", you can say
> something maybe like "createdb-only" because this option only applies
> to CREATE DATABASE related commands, not CREATE TABLE or other
> objects. In the help menu, you can then elaborate more that this
> option "dump only the commands related to create database like ALTER,
> GRANT..etc"
Well I have to say I agree with Peter that the option name I came up
with is pretty confusing, not sure createdb-only is much better as it
also includes GRANTs etc.
I think from a technical POV it's useful as it closes a gap between
pg_dumpall -g and pg_dump -Fc $DATABASE in my opinion, without having to
potentially schema-dump and filter out a large number of database
objects.
Anybody else have some opinions on what to call this best? Maybe just a
short option and some explanatory text in --help along with it?
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.banck@credativ.de
credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
pgsql-hackers by date: