From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: file cloning in pg_upgrade and CREATE DATABASE
Date: Wed, 7 Nov 2018 18:42:00 +0100
Message-ID: <98287e67-0f94-d962-e550-bd169a737181@2ndquadrant.com> (raw)
In-Reply-To: <20181018235046.GA2099@paquier.xyz>
References: <20181003014409.GC2609@paquier.xyz>
<20181003020706.hykmk4vmy6brovoj@alvherre.pgsql>
<20181010195002.GA7806@momjian.us>
<325b5baf-adcc-9954-2a55-d67b4f3155f0@2ndquadrant.com>
<d4bc3dae-fdea-2407-5629-cac68be893a3@2ndquadrant.com>
<20181018235046.GA2099@paquier.xyz>
On 19/10/2018 01:50, Michael Paquier wrote:
> On Thu, Oct 18, 2018 at 11:59:00PM +0200, Peter Eisentraut wrote:
>> New patch that removes all the various reflink modes and adds a new
>> option --clone that works similar to --link. I think it's much cleaner now.
>
> Thanks Peter for the new version.
>
> +
> + {"clone", no_argument, NULL, 1},
> +
> {NULL, 0, NULL, 0}
>
> Why newlines here?
fixed
> @@ -293,6 +300,7 @@ usage(void)
> printf(_(" -U, --username=NAME cluster superuser (default \"%s\")\n"), os_info.user);
> printf(_(" -v, --verbose enable verbose internal logging\n"));
> printf(_(" -V, --version display version information, then exit\n"));
> + printf(_(" --clone clone instead of copying files to new cluster\n"));
> printf(_(" -?, --help show this help, then exit\n"));
> printf(_("\n"
>
> An idea for a one-letter option could be -n. This patch can live
> without.
-n is often used for something like "dry run", so it didn't go for that
here. I suspect the cloning will remain a marginal option for some
time, so having only a long option is acceptable.
> + pg_fatal("error while cloning relation \"%s.%s\": could not open file \"%s\": %s\n",
> + schemaName, relName, src, strerror(errno));
>
> The tail of those error messages "could not open file" and "could not
> create file" are already available as translatable error messages.
> Would it be better to split those messages in two for translators? One
> is generated with pg_fatal("error while cloning relation \"%s.%s\":
> could not open file \"%s\": %s\n",
> + schemaName, relName, src, strerror(errno));
I think this is too complicated for a few messages.
> Those are all minor points, the patch looks good to me.
Committed, thanks.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, michael@paquier.xyz, bruce@momjian.us, alvherre@2ndquadrant.com
Subject: Re: file cloning in pg_upgrade and CREATE DATABASE
In-Reply-To: <98287e67-0f94-d962-e550-bd169a737181@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox