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.

3 files changed, 13 insertions, 14 deletions

diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 9b25dc5b284..4f27ce713f5 100644
--- 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 5b0014210f7..10182de949f 100644
--- 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 0c1a8220bbf..85cb717f748 100644
--- 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);