pg_upgrade: Don't overwrite existing files.

For historical reasons, copyFile and rewriteVisibilityMap took a force
argument which was always passed as true, meaning that any existing
file should be overwritten.  However, it seems much safer to instead
fail if a file we need to write already exists.

While we're at it, remove the "force" argument altogether, since it was
never passed as anything other than true (and now we would never pass
it as anything other than false, if we kept it).

Noted by Andres Freund during post-commit review of the patch that added
rewriteVisibilityMap, commit 7087166a88fe0c04fc6636d0d6d6bea1737fc1fb,
but this also changes the behavior when copying files without rewriting
them.

Patch by Masahiko Sawada.

diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 9b25dc5b284bbd5899eedaa9294f78cbf778b56f..4f27ce713f5659b47db822a6a523a38f5c27674e 100644 (file)
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -22,7 +22,7 @@
 
 
 #ifndef WIN32
-static int     copy_file(const char *fromfile, const char *tofile, bool force);
+static int     copy_file(const char *fromfile, const char *tofile);
 #else
 static int     win32_pghardlink(const char *src, const char *dst);
 #endif
@@ -34,12 +34,12 @@ static int  win32_pghardlink(const char *src, const char *dst);
  *     Copies a relation file from src to dst.
  */
 const char *
-copyFile(const char *src, const char *dst, bool force)
+copyFile(const char *src, const char *dst)
 {
 #ifndef WIN32
-               if (copy_file(src, dst, force) == -1)
+               if (copy_file(src, dst) == -1)
 #else
-               if (CopyFile(src, dst, !force) == 0)
+               if (CopyFile(src, dst, true) == 0)
 #endif
                        return getErrorText();
                else
@@ -68,7 +68,7 @@ linkFile(const char *src, const char *dst)
 
 #ifndef WIN32
 static int
-copy_file(const char *srcfile, const char *dstfile, bool force)
+copy_file(const char *srcfile, const char *dstfile)
 {
 #define COPY_BUF_SIZE (50 * BLCKSZ)
 
@@ -87,7 +87,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force)
        if ((src_fd = open(srcfile, O_RDONLY, 0)) < 0)
                return -1;
 
-       if ((dest_fd = open(dstfile, O_RDWR | O_CREAT | (force ? 0 : O_EXCL), S_IRUSR | S_IWUSR)) < 0)
+       if ((dest_fd = open(dstfile, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR)) < 0)
        {
                save_errno = errno;
 
@@ -159,7 +159,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force)
  * VACUUM.
  */
 const char *
-rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force)
+rewriteVisibilityMap(const char *fromfile, const char *tofile)
 {
        int                     src_fd = 0;
        int                     dst_fd = 0;
@@ -186,7 +186,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force)
                return getErrorText();
        }
 
-       if ((dst_fd = open(tofile, O_RDWR | O_CREAT | (force ? 0 : O_EXCL), S_IRUSR | S_IWUSR)) < 0)
+       if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR)) < 0)
        {
                close(src_fd);
                return getErrorText();

diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 5b0014210f794cce38423a9c29857647f5bb4d15..10182de949fd7733bc0f1bd6b6fbedeae80dffe1 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -367,10 +367,9 @@ bool               pid_lock_file_exists(const char *datadir);
 
 /* file.c */
 
-const char *copyFile(const char *src, const char *dst, bool force);
+const char *copyFile(const char *src, const char *dst);
 const char *linkFile(const char *src, const char *dst);
-const char *rewriteVisibilityMap(const char *fromfile, const char *tofile,
-                                                                bool force);
+const char *rewriteVisibilityMap(const char *fromfile, const char *tofile);
 
 void           check_hard_link(void);
 FILE      *fopen_priv(const char *path, const char *mode);

diff --git a/src/bin/pg_upgrade/relfilenode.c b/src/bin/pg_upgrade/relfilenode.c
index 0c1a8220bbfba61b1a9264c1b4ff19a1d14b4c28..85cb717f748779cd15d95abe9702b17899c61490 100644 (file)
--- a/src/bin/pg_upgrade/relfilenode.c
+++ b/src/bin/pg_upgrade/relfilenode.c
@@ -248,9 +248,9 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
 
                        /* Rewrite visibility map if needed */
                        if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
-                               msg = rewriteVisibilityMap(old_file, new_file, true);
+                               msg = rewriteVisibilityMap(old_file, new_file);
                        else
-                               msg = copyFile(old_file, new_file, true);
+                               msg = copyFile(old_file, new_file);
 
                        if (msg)
                                pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
@@ -262,7 +262,7 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
 
                        /* Rewrite visibility map if needed */
                        if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
-                               msg = rewriteVisibilityMap(old_file, new_file, true);
+                               msg = rewriteVisibilityMap(old_file, new_file);
                        else
                                msg = linkFile(old_file, new_file);