Re: file cloning in pg_upgrade and CREATE DATABASE - Mailing list pgsql-hackers

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?

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

+     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));

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_VERBOSE to mention that cloning checks are
done, and the second one is fatal with the actual errors.

Those are all minor points, the patch looks good to me.
--
Michael