Обсуждение: HAVE_WORKING_LINK still needed?

I came across the HAVE_WORKING_LINK define in pg_config_manual.h. 
AFAICT, hard links are supported on Windows and Cygwin in the OS 
versions that we support, and pg_upgrade already contains the required 
shim.  It seems to me we could normalize and simplify that, as in the 
attached patches.  (Perhaps rename durable_link_or_rename() then.)  I 
successfully tested on MSVC, MinGW, and Cygwin.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-Feb-28, Peter Eisentraut wrote:

> @@ -788,7 +788,6 @@ durable_link_or_rename(const char *oldfile, const char *newfile, int elevel)
>      if (fsync_fname_ext(oldfile, false, false, elevel) != 0)
>          return -1;
>  
> -#ifdef HAVE_WORKING_LINK
>      if (link(oldfile, newfile) < 0)
>      {
>          ereport(elevel,
> @@ -798,17 +797,6 @@ durable_link_or_rename(const char *oldfile, const char *newfile, int elevel)
>          return -1;
>      }
>      unlink(oldfile);
> -#else
> -    /* XXX: Add racy file existence check? */
> -    if (rename(oldfile, newfile) < 0)

Maybe rename durable_link_or_rename to just durable_link?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I came across the HAVE_WORKING_LINK define in pg_config_manual.h. 
> AFAICT, hard links are supported on Windows and Cygwin in the OS 
> versions that we support, and pg_upgrade already contains the required 
> shim.  It seems to me we could normalize and simplify that, as in the 
> attached patches.  (Perhaps rename durable_link_or_rename() then.)  I 
> successfully tested on MSVC, MinGW, and Cygwin.

I don't have any way to test on Windows, but this patchset passes
eyeball review.  +1 for getting rid of the special cases.
Also +1 for s/durable_link_or_rename/durable_link/.

            regards, tom lane

On 2020-Feb-28, Tom Lane wrote:

> Also +1 for s/durable_link_or_rename/durable_link/.

Actually, it's not *that* either, because what the function does is link
followed by unlink.  So it's more a variation of durable_rename with
slightly different semantics -- the difference is what happens if a file
with the target name already exists.  Maybe call it durable_rename_no_overwrite.

There's a lot of commonality between the two.  Perhaps it's not entirely
silly to merge both as a single routine, with a flag to select either
behavior.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-02-28 19:44, Alvaro Herrera wrote:
> On 2020-Feb-28, Tom Lane wrote:
> 
>> Also +1 for s/durable_link_or_rename/durable_link/.
> 
> Actually, it's not *that* either, because what the function does is link
> followed by unlink.  So it's more a variation of durable_rename with
> slightly different semantics -- the difference is what happens if a file
> with the target name already exists.  Maybe call it durable_rename_no_overwrite.

I have committed the first two patches.

Here is the third patch again, we renaming durable_link_or_rename() to 
durable_rename_excl().  This seems to match existing Unix system call 
naming best (see open() flag O_EXCL, and macOS has a renamex_np() flag 
RENAME_EXCL).

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-03-04 17:37, Peter Eisentraut wrote:
> Here is the third patch again, we renaming durable_link_or_rename() to
> durable_rename_excl().  This seems to match existing Unix system call
> naming best (see open() flag O_EXCL, and macOS has a renamex_np() flag
> RENAME_EXCL).

committed like that

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services