Commit e191a690 authored by Robert Haas's avatar Robert Haas

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 7087166a,
but this also changes the behavior when copying files without rewriting
them.

Patch by Masahiko Sawada.
parent 932b97a0
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
#ifndef WIN32 #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 #else
static int win32_pghardlink(const char *src, const char *dst); static int win32_pghardlink(const char *src, const char *dst);
#endif #endif
...@@ -34,12 +34,12 @@ static int win32_pghardlink(const char *src, const char *dst); ...@@ -34,12 +34,12 @@ static int win32_pghardlink(const char *src, const char *dst);
* Copies a relation file from src to dst. * Copies a relation file from src to dst.
*/ */
const char * const char *
copyFile(const char *src, const char *dst, bool force) copyFile(const char *src, const char *dst)
{ {
#ifndef WIN32 #ifndef WIN32
if (copy_file(src, dst, force) == -1) if (copy_file(src, dst) == -1)
#else #else
if (CopyFile(src, dst, !force) == 0) if (CopyFile(src, dst, true) == 0)
#endif #endif
return getErrorText(); return getErrorText();
else else
...@@ -68,7 +68,7 @@ linkFile(const char *src, const char *dst) ...@@ -68,7 +68,7 @@ linkFile(const char *src, const char *dst)
#ifndef WIN32 #ifndef WIN32
static int 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) #define COPY_BUF_SIZE (50 * BLCKSZ)
...@@ -87,7 +87,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force) ...@@ -87,7 +87,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force)
if ((src_fd = open(srcfile, O_RDONLY, 0)) < 0) if ((src_fd = open(srcfile, O_RDONLY, 0)) < 0)
return -1; 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; save_errno = errno;
...@@ -159,7 +159,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force) ...@@ -159,7 +159,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force)
* VACUUM. * VACUUM.
*/ */
const char * const char *
rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) rewriteVisibilityMap(const char *fromfile, const char *tofile)
{ {
int src_fd = 0; int src_fd = 0;
int dst_fd = 0; int dst_fd = 0;
...@@ -186,7 +186,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) ...@@ -186,7 +186,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force)
return getErrorText(); 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); close(src_fd);
return getErrorText(); return getErrorText();
......
...@@ -367,10 +367,9 @@ bool pid_lock_file_exists(const char *datadir); ...@@ -367,10 +367,9 @@ bool pid_lock_file_exists(const char *datadir);
/* file.c */ /* 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 *linkFile(const char *src, const char *dst);
const char *rewriteVisibilityMap(const char *fromfile, const char *tofile, const char *rewriteVisibilityMap(const char *fromfile, const char *tofile);
bool force);
void check_hard_link(void); void check_hard_link(void);
FILE *fopen_priv(const char *path, const char *mode); FILE *fopen_priv(const char *path, const char *mode);
......
...@@ -248,9 +248,9 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro ...@@ -248,9 +248,9 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
/* Rewrite visibility map if needed */ /* Rewrite visibility map if needed */
if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0)) if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
msg = rewriteVisibilityMap(old_file, new_file, true); msg = rewriteVisibilityMap(old_file, new_file);
else else
msg = copyFile(old_file, new_file, true); msg = copyFile(old_file, new_file);
if (msg) if (msg)
pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n", 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 ...@@ -262,7 +262,7 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
/* Rewrite visibility map if needed */ /* Rewrite visibility map if needed */
if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0)) if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
msg = rewriteVisibilityMap(old_file, new_file, true); msg = rewriteVisibilityMap(old_file, new_file);
else else
msg = linkFile(old_file, new_file); msg = linkFile(old_file, new_file);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment