From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Subject: | Re: file cloning in pg_upgrade and CREATE DATABASE |
Date: | 2018-11-07 17:42:00 |
Message-ID: | 98287e67-0f94-d962-e550-bd169a737181@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2018-11-07 17:44:03 | Does slot_deform_tuple need to care about dropped columns? |
Previous Message | Tom Lane | 2018-11-07 17:17:57 | Re: Calculate total_table_pages after set_base_rel_sizes() |