Re: Regression test PANICs with master-standby setup on samemachine

Hello.

Win32 implement cannot have symbolic link feature as Linux-like
OSes for some restrictions. (Windows 7 and 10 behave differently,
as I heard.)

So the 0002 patch implemnets "fake" symbolic link as mentioned in
its commit message.

Also I fixed 0001 slightly.

regards.

At Thu, 25 Apr 2019 17:08:55 +0900 (Tokyo Standard Time), Kyotaro
HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
> Thanks for the suggestions, Tom, Andres. For clarity, as I
> mentioned in the post, I didn't like the getcwd in 0001. The
> reason for the previous patch are:
> 
> 1. canonicalize_path doesn't process '/.' and '/..' in the middle
>   of a path. That prevents correct checking of directory
>   inclusiveness. Actually regression test suffers that.
> 
> 2. Simply I missed make_absolute_path..
> 
> So, I rewote canonicalize_path to process '.' and '..'s not only
> at the end of a path but all occurrances in a path. This makes
> the strange loop of chdir-getwen useless. But the new
> canonicalize_path is a bit complex.
> 
> The modified canonicalize_path works filesystem-access-free so it
> doesn't follow symlinks. Thus it makes a misjudge when two paths
> are in an inclusion relationship after resolving symlinks in the
> paths. But I don't think we don't need to treat such a malicious
> situation.

reagrds.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 762ec1cbea1a6a11b6f4569549611a7c2c31a3dc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 25 Apr 2019 15:08:54 +0900
Subject: [PATCH 1/2] Allow relative tablespace location paths

Currently relative paths are not allowed as tablespace directory. That
makes tests about tablespaces bothersome but doesn't prevent them
points into the data directory perfectly. This patch allows relative
direcotry based on the data directory as tablespace directory. Then
makes the check more strict.
---
 src/backend/access/transam/xlog.c            |   8 ++
 src/backend/commands/tablespace.c            |  55 ++++++--
 src/backend/replication/basebackup.c         |  11 +-
 src/backend/utils/adt/misc.c                 |  10 +-
 src/bin/pg_basebackup/pg_basebackup.c        | 105 +++++++++++---
 src/bin/pg_basebackup/t/010_pg_basebackup.pl |  18 ++-
 src/port/path.c                              | 204 +++++++++++++++++----------
 7 files changed, 292 insertions(+), 119 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c00b63c751..abc2fd951f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10419,6 +10419,14 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
             }
             linkpath[rllen] = '\0';
 
+            /*
+             * In the relative path cases, the link target is always prefixed
+             * by "../" to convert from data directory-based. So we just do
+             * the reverse here.
+             */
+            if (strncmp(s, "../", 3) == 0)
+                s += 3;
+
             /*
              * Add the escape character '\\' before newline in a string to
              * ensure that we can distinguish between the newline in the
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 8ec963f1cf..fada719687 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -94,6 +94,25 @@ static void create_tablespace_directories(const char *location,
                               const Oid tablespaceoid);
 static bool destroy_tablespace_directories(Oid tablespaceoid, bool redo);
 
+/*
+ * Check if the path is in the data directory strictly.
+ */
+static bool
+is_in_data_directory(const char *path)
+{
+    char   *tmp_path = make_absolute_path(path);
+    char    tmp_datadir[MAXPGPATH];
+    bool    in_datadir;
+
+    Assert (tmp_path != NULL);
+    strlcpy(tmp_datadir, DataDir, MAXPGPATH);
+    canonicalize_path(tmp_datadir);
+
+    in_datadir = path_is_prefix_of_path(tmp_datadir, tmp_path);
+    free(tmp_path);
+
+    return in_datadir;
+}
 
 /*
  * Each database using a table space is isolated into its own name space
@@ -272,30 +291,26 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
      *
      * this also helps us ensure that location is not empty or whitespace
      */
-    if (!is_absolute_path(location))
+    /* Reject tablespaces in the data directory. */
+    if (is_in_data_directory(location))
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                 errmsg("tablespace location must be an absolute path")));
+                 errmsg("tablespace location must not be inside the data directory")));
 
     /*
      * Check that location isn't too long. Remember that we're going to append
-     * 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'.  FYI, we never actually
+     * 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'.  In the relative path case, we
+     * attach "../" at the beginning of the path. FYI, we never actually
      * reference the whole path here, but MakePGDirectory() uses the first two
      * parts.
      */
-    if (strlen(location) + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 +
+    if (3 + strlen(location) + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 +
         OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1 + OIDCHARS > MAXPGPATH)
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                  errmsg("tablespace location \"%s\" is too long",
                         location)));
 
-    /* Warn if the tablespace is in the data directory. */
-    if (path_is_prefix_of_path(DataDir, location))
-        ereport(WARNING,
-                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                 errmsg("tablespace location should not be inside the data directory")));
-
     /*
      * Disallow creation of tablespaces named "pg_xxx"; we reserve this
      * namespace for system purposes.
@@ -570,8 +585,10 @@ static void
 create_tablespace_directories(const char *location, const Oid tablespaceoid)
 {
     char       *linkloc;
+    char       *linktarget;
     char       *location_with_version_dir;
-    struct stat st;
+    bool        free_linktarget = false;
+     struct stat st;
 
     linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
     location_with_version_dir = psprintf("%s/%s", location,
@@ -639,13 +656,27 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 
     /*
      * Create the symlink under PGDATA
+     * Relative tablespace location is based on PGDATA directory. The symlink
+     * is created in PGDATA/pg_tblspc. So adjust relative paths by attaching
+     * "../" at the head.
      */
-    if (symlink(location, linkloc) < 0)
+    if (is_absolute_path(location))
+        linktarget = unconstify(char *, location);
+    else
+    {
+        linktarget = psprintf("../%s", location);
+        free_linktarget = true;
+    }
+
+    if (symlink(linktarget, linkloc) < 0)
         ereport(ERROR,
                 (errcode_for_file_access(),
                  errmsg("could not create symbolic link \"%s\": %m",
                         linkloc)));
 
+    if (free_linktarget)
+        pfree(linktarget);
+
     pfree(linkloc);
     pfree(location_with_version_dir);
 }
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 36dcb28754..98e34e21c1 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1224,6 +1224,7 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 #if defined(HAVE_READLINK) || defined(WIN32)
             char        linkpath[MAXPGPATH];
             int            rllen;
+            int            linkoffset = 0;
 
             rllen = readlink(pathbuf, linkpath, sizeof(linkpath));
             if (rllen < 0)
@@ -1238,7 +1239,15 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
                                 pathbuf)));
             linkpath[rllen] = '\0';
 
-            size += _tarWriteHeader(pathbuf + basepathlen + 1, linkpath,
+            /*
+             * Relative link target is always prefixed by "../". Remove it to
+             * obtain a tablespace directory relative to the data directory.
+             */
+            if (strncmp(linkpath, "../", 3) == 0)
+                linkoffset = 3;
+
+            size += _tarWriteHeader(pathbuf + basepathlen + 1,
+                                    linkpath + linkoffset,
                                     &statbuf, sizeonly);
 #else
 
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index d330a88e3c..205c8ac903 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -293,6 +293,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
     char        sourcepath[MAXPGPATH];
     char        targetpath[MAXPGPATH];
     int            rllen;
+    int            pathoffset = 0;
 
     /*
      * It's useful to apply this function to pg_class.reltablespace, wherein
@@ -330,7 +331,14 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
                         sourcepath)));
     targetpath[rllen] = '\0';
 
-    PG_RETURN_TEXT_P(cstring_to_text(targetpath));
+    /*
+     * Relative target paths are always prefixed by "../". Remove it to obtain
+     * tablespace directory relative to the data directory.
+     */
+    if (strncmp(targetpath, "../", 3) == 0)
+        pathoffset = 3;
+
+    PG_RETURN_TEXT_P(cstring_to_text(targetpath + pathoffset));
 #else
     ereport(ERROR,
             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 1a735b8046..428e2340cc 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -269,26 +269,6 @@ tablespace_list_append(const char *arg)
         exit(1);
     }
 
-    /*
-     * This check isn't absolutely necessary.  But all tablespaces are created
-     * with absolute directories, so specifying a non-absolute path here would
-     * just never match, possibly confusing users.  It's also good to be
-     * consistent with the new_dir check.
-     */
-    if (!is_absolute_path(cell->old_dir))
-    {
-        pg_log_error("old directory is not an absolute path in tablespace mapping: %s",
-                     cell->old_dir);
-        exit(1);
-    }
-
-    if (!is_absolute_path(cell->new_dir))
-    {
-        pg_log_error("new directory is not an absolute path in tablespace mapping: %s",
-                     cell->new_dir);
-        exit(1);
-    }
-
     /*
      * Comparisons done with these values should involve similarly
      * canonicalized path values.  This is particularly sensitive on Windows
@@ -1399,9 +1379,15 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
     if (basetablespace)
         strlcpy(current_path, basedir, sizeof(current_path));
     else
-        strlcpy(current_path,
-                get_tablespace_mapping(PQgetvalue(res, rownum, 1)),
-                sizeof(current_path));
+    {
+        const char *path = get_tablespace_mapping(PQgetvalue(res, rownum, 1));
+
+        /* Relative tablespace locations need to be prefixed by basedir. */
+        if (is_absolute_path(path))
+            strlcpy(current_path, path, sizeof(current_path));
+        else
+            join_path_components(currennt_path, basedir, path);
+    }
 
     /*
      * Get the COPY data
@@ -1525,15 +1511,35 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
                      * are, you can call it an undocumented feature that you
                      * can map them too.)
                      */
+                    bool free_path = false;
+
                     filename[strlen(filename) - 1] = '\0';    /* Remove trailing slash */
 
                     mapped_tblspc_path = get_tablespace_mapping(©buf[157]);
+                    if (!is_absolute_path(mapped_tblspc_path))
+                    {
+                        /*
+                         * Convert relative tablespace location based on data
+                         * directory into path link target based on pg_tblspc
+                         * directory. This must be in sync with
+                         * create_tablespace_directories().
+                         */
+                        char *p = malloc(3 + strlen(mapped_tblspc_path) + 1);
+
+                        strcpy(p, "../");
+                        strcat(p, mapped_tblspc_path);
+                        mapped_tblspc_path = p;
+                        free_path = true;
+                    }
                     if (symlink(mapped_tblspc_path, filename) != 0)
                     {
                         pg_log_error("could not create symbolic link from \"%s\" to \"%s\": %m",
                                      filename, mapped_tblspc_path);
                         exit(1);
                     }
+
+                    if (free_path)
+                        free(unconstify(char *, mapped_tblspc_path));
                 }
                 else
                 {
@@ -1957,8 +1963,25 @@ BaseBackup(void)
         if (format == 'p' && !PQgetisnull(res, i, 1))
         {
             char       *path = unconstify(char *, get_tablespace_mapping(PQgetvalue(res, i, 1)));
+            bool        freeit = false;
+
+            if (!is_absolute_path(path))
+            {
+                /*
+                 * Relative tablespace locations need to be converted
+                 * attaching basedir.
+                 */
+                char *p = malloc(strlen(basedir) + 1 + strlen(path) + 1);
+
+                join_path_components(p, basedir, path);
+                path = p;
+                freeit = true;
+            }
 
             verify_dir_is_empty_or_create(path, &made_tablespace_dirs, &found_tablespace_dirs);
+
+            if (freeit)
+                free(path);
         }
     }
 
@@ -2163,6 +2186,39 @@ BaseBackup(void)
         pg_log_info("base backup completed");
 }
 
+/* Sanity check of tablespace mappings */
+static void
+check_tablespace_mappings(void)
+{
+    TablespaceListCell *cell;
+    char *absbasedir = make_absolute_path(basedir);
+
+    for (cell = tablespace_dirs.head ; cell ; cell = cell->next)
+    {
+        char pathbuf[MAXPGPATH];
+        char *path = cell->new_dir;
+
+        /*
+         * absbasedir doesn't have trailing '/'. new_dir is already
+         * canonicalized.
+         */
+        if (!is_absolute_path(cell->new_dir))
+        {
+            /* but concatenated path needs re-canonicalization */
+            snprintf(pathbuf, MAXPGPATH, "%s/%s", absbasedir, cell->new_dir);
+            canonicalize_path(pathbuf);
+            path = pathbuf;
+        }
+
+        if (path_is_prefix_of_path(absbasedir, path))
+        {
+            pg_log_error("new_dir \"%s\" of mapping for \"%s\" is inside target data directory \"%s\"", cell->new_dir,
cell->old_dir,absbasedir);
 
+            exit(1);
+        }
+    }
+
+    free(absbasedir);
+}
 
 int
 main(int argc, char **argv)
@@ -2478,6 +2534,9 @@ main(int argc, char **argv)
         }
     }
 
+    /* Sanity check of tablespace mapping */
+    check_tablespace_mappings();
+
 #ifndef HAVE_LIBZ
     if (compresslevel != 0)
     {
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 33869fecc9..239569a4c3 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -6,7 +6,7 @@ use File::Basename qw(basename dirname);
 use File::Path qw(rmtree);
 use PostgresNode;
 use TestLib;
-use Test::More tests => 106;
+use Test::More tests => 108;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -186,12 +186,20 @@ $node->command_fails(
         "-T/foo=/bar=/baz"
     ],
     '-T with multiple = fails');
+$node->command_ok(
+    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo=../bar/baz/foo" ],
+    '-T with relative old directory not rejected');
+rmtree("$tempdir/backup_foo");
 $node->command_fails(
-    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo=/bar" ],
-    '-T with old directory not absolute fails');
+    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=../backup_foo/bar/baz" ],
+    '-T with new relative directory inside data dir fails');
+$node->command_ok(
+    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=/foo/bar" ],
+    '-T with new directory under nonexistent directory succeeds');
+rmtree("$tempdir/backup_foo");
 $node->command_fails(
-    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=bar" ],
-    '-T with new directory not absolute fails');
+    [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=../backup_foo/bar/baz" ],
+    '-T with new relative directory inside data dir fails');
 $node->command_fails(
     [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo" ],
     '-T with invalid format fails');
diff --git a/src/port/path.c b/src/port/path.c
index 4b214e89e4..dd8ae40e0f 100644
--- a/src/port/path.c
+++ b/src/port/path.c
@@ -247,19 +247,17 @@ join_path_components(char *ret_path,
  *        o  remove trailing quote on Win32
  *        o  remove trailing slash
  *        o  remove duplicate adjacent separators
- *        o  remove trailing '.'
- *        o  process trailing '..' ourselves
+ *        o  process '.' and '..' ourselves
  */
 void
 canonicalize_path(char *path)
 {
-    char       *p,
-               *to_p;
-    char       *spath;
-    bool        was_sep = false;
-    int            pending_strips;
+    char    *src;
+    char    *dst;
+    int     totallen PG_USED_FOR_ASSERTS_ONLY;
 
 #ifdef WIN32
+    char    *p;
 
     /*
      * The Windows command processor will accept suitably quoted paths with
@@ -277,91 +275,143 @@ canonicalize_path(char *path)
      */
     if (p > path && *(p - 1) == '"')
         *(p - 1) = '/';
+
+    /* Skip drive component or UNC header */
+    path = skip_drive(path);
+#endif
+
+#ifdef USE_ASSERT_CHECKING
+    totallen = strlen(path);
 #endif
 
     /*
-     * Removing the trailing slash on a path means we never get ugly double
-     * trailing slashes. Also, Win32 can't stat() a directory with a trailing
-     * slash. Don't remove a leading slash, though.
-     */
-    trim_trailing_separator(path);
-
-    /*
-     * Remove duplicate adjacent separators
-     */
-    p = path;
-#ifdef WIN32
-    /* Don't remove leading double-slash on Win32 */
-    if (*p)
-        p++;
-#endif
-    to_p = p;
-    for (; *p; p++, to_p++)
-    {
-        /* Handle many adjacent slashes, like "/a///b" */
-        while (*p == '/' && was_sep)
-            p++;
-        if (to_p != p)
-            *to_p = *p;
-        was_sep = (*p == '/');
-    }
-    *to_p = '\0';
-
-    /*
-     * Remove any trailing uses of "." and process ".." ourselves
+     * Process any uses of "." and ".." ourselves
      *
      * Note that "/../.." should reduce to just "/", while "../.." has to be
-     * kept as-is.  In the latter case we put back mistakenly trimmed ".."
-     * components below.  Also note that we want a Windows drive spec to be
-     * visible to trim_directory(), but it's not part of the logic that's
-     * looking at the name components; hence distinction between path and
-     * spath.
+     * kept as-is.
      */
-    spath = skip_drive(path);
-    pending_strips = 0;
-    for (;;)
-    {
-        int            len = strlen(spath);
+    src = dst = path;
 
-        if (len >= 2 && strcmp(spath + len - 2, "/.") == 0)
-            trim_directory(path);
-        else if (strcmp(spath, ".") == 0)
+    while (*src)
+    {
+        char *p;
+        int elen;
+
+        Assert(dst >= path && src >= path &&
+               dst - path <= totallen && src - path <= totallen);
+
+        /* copy separator, removing duplicate adjacent separators */
+        if (IS_DIR_SEP(*src))
         {
-            /* Want to leave "." alone, but "./.." has to become ".." */
-            if (pending_strips > 0)
-                *spath = '\0';
-            break;
+            if (dst > path && IS_DIR_SEP(dst[-1]))
+            {
+                /* skip duplicates */
+                src++;
+                continue;
+            }
+            *dst++ = *src++;
+            continue;
         }
-        else if ((len >= 3 && strcmp(spath + len - 3, "/..") == 0) ||
-                 strcmp(spath, "..") == 0)
+
+        /* locate the next element */
+        for (p = src ; *p && !IS_DIR_SEP(*p) ; p++);
+        elen = p - src;
+
+        Assert (elen != 0);
+
+        /* process "." */
+        if (elen == 1 && src[0] == '.')
         {
-            trim_directory(path);
-            pending_strips++;
+            /* just skip the element and succeeding separators */
+            for (src += elen ; IS_DIR_SEP(*src) ; src++);
+
+            /* If the path was exactly ".", don't skip it */
+            if (*src || src != path + 1)
+                continue;
+
+            src = path;
         }
-        else if (pending_strips > 0 && *spath != '\0')
+        /* process ".." not at the beginning of a relative path */
+        else if (elen == 2 && src[0] == '.' && src[1] == '.' && dst != path)
         {
-            /* trim a regular directory name canceled by ".." */
-            trim_directory(path);
-            pending_strips--;
-            /* foo/.. should become ".", not empty */
-            if (*spath == '\0')
-                strcpy(spath, ".");
+            /*
+             * Found ".." and there's something up-step, step back.
+             * But only when we have the upper element to back up to.
+             * Also don't annihilate two successive ".."s.
+             */
+            if (dst == path + 1)
+            {
+                /*
+                 * dst is absolute root. skip the ".." and trailing separator
+                 * if any
+                 */
+                src += elen;
+                if (IS_DIR_SEP(*src))
+                    src++;
+                continue;
+            }
+            else
+            {
+                /* Find the previous element. dst[-1] == '/' here.  */
+                char *pelm;
+
+                for (pelm = dst - 1 ;
+                     pelm > path && !IS_DIR_SEP(pelm[-1]) ;
+                     pelm--);
+
+                if (dst - pelm == 2 && *pelm == '.')
+                {
+                    /*
+                     * Convert "./.." into ".." then process this ".."
+                     * again.
+                     */
+                    dst = pelm;
+                    continue;
+                }
+                else if (dst - pelm > 3 || strncmp(pelm, "..", 2) != 0)
+                {
+                    /* The previous element is not a relative movement */
+                    if (pelm == path)
+                    {
+                        /*
+                         * The relative path is being translated as
+                         * nothing. Insert "./" instead.
+                         */
+                        memcpy(path, "./", 2);
+                        dst = path + 2;
+                        src += elen;
+                        if (IS_DIR_SEP(*src))
+                            src++;
+                    }
+                    else
+                    {
+                        /* annihilate involving the previous element */
+                        dst = pelm;
+                        src += elen;
+                        if (IS_DIR_SEP(*src))
+                            src++;
+                    }
+                    continue;
+                }
+            }
         }
-        else
-            break;
+        else if ((dst == path + 2 && *path == '.') ||
+                 (dst > path + 2 && IS_DIR_SEP(dst[-3]) && dst[-2] == '.'))
+        {
+            /* the previous element is removable ".", eliminate it */
+            dst -= 2;
+        }
+
+        /* copy it, then move */
+        memcpy(dst, src, elen);
+        dst += elen;
+        src += elen;
     }
 
-    if (pending_strips > 0)
-    {
-        /*
-         * We could only get here if path is now totally empty (other than a
-         * possible drive specifier on Windows). We have to put back one or
-         * more ".."'s that we took off.
-         */
-        while (--pending_strips > 0)
-            strcat(path, "../");
-        strcat(path, "..");
-    }
+    /* trim trailing separators */
+    while (dst > path + 1 && IS_DIR_SEP(dst[-1]))
+        dst--;
+    *dst = 0;
 }
 
 /*
-- 
2.16.3

From e89a60f0451f1c63f53d3ecb609556785ce44930 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Fri, 26 Apr 2019 17:19:01 +0900
Subject: [PATCH 2/2] win32 tentative implement

This is a patch for discussion.

Win32 implement doesn't have relative symbolic links. AFAICS relative
symbolic links on Win32 requires special setting to use
CreateSymbolicLink with non-administrator accounts, and there's no way
to use DeviceIoControl for IO_REPARSE_TAG_SYMLINK with non-admistrator
accounts.

So, this patch creates "fake" symbolic links, which is, it's really a
mount point but looks as if a symbolic links. The difference is
revealed (that is, the "relative" link will be broken) when moving the
data directory and tablespace directory to another place.
---
 src/port/dirmod.c | 75 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 58 insertions(+), 17 deletions(-)

diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index d7932401ef..f8aaae2c64 100644
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -159,10 +159,13 @@ int
 pgsymlink(const char *oldpath, const char *newpath)
 {
     HANDLE        dirhandle;
-    DWORD        len;
-    char        buffer[MAX_PATH * sizeof(WCHAR) + offsetof(REPARSE_JUNCTION_DATA_BUFFER, PathBuffer)];
+    DWORD        len, plen;
+    char        buffer[MAX_PATH * sizeof(WCHAR) * 2 + offsetof(REPARSE_JUNCTION_DATA_BUFFER, PathBuffer)];
     char        nativeTarget[MAX_PATH];
+    char        nativePrint[MAX_PATH];
+    char        tmp[MAX_PATH];
     char       *p = nativeTarget;
+    char       *poldpath = oldpath;
     REPARSE_JUNCTION_DATA_BUFFER *reparseBuf = (REPARSE_JUNCTION_DATA_BUFFER *) buffer;
 
     CreateDirectory(newpath, 0);
@@ -173,25 +176,50 @@ pgsymlink(const char *oldpath, const char *newpath)
     if (dirhandle == INVALID_HANDLE_VALUE)
         return -1;
 
-    /* make sure we have an unparsed native win32 path */
-    if (memcmp("\\??\\", oldpath, 4) != 0)
-        snprintf(nativeTarget, sizeof(nativeTarget), "\\??\\%s", oldpath);
+    /* make absolute path if oldpath is not */
+    if (!is_absolute_path(poldpath))
+    {
+      char *absnew = make_absolute_path(newpath);
+      /* back up to pg_tblspc */
+      get_parent_directory(absnew);
+      join_path_components(tmp, absnew, oldpath);
+      free(absnew);
+
+ canonicalize_path(tmp);
+
+      /* Copy oldpath as print name */
+      strlcpy(nativePrint, oldpath, sizeof(nativePrint));
+
+      poldpath = tmp;
+    }
     else
-        strlcpy(nativeTarget, oldpath, sizeof(nativeTarget));
+      nativePrint[0] = 0;
+
+    /* make sure we have an unparsed native win32 path */
+    if (is_absolute_path(poldpath))
+    {
+        if (memcmp("\\??\\", poldpath, 4) != 0)
+          snprintf(nativeTarget, sizeof(nativeTarget), "\\??\\%s", poldpath);
+        else
+          strlcpy(nativeTarget, poldpath, sizeof(nativeTarget));
+    }
 
     while ((p = strchr(p, '/')) != NULL)
         *p++ = '\\';
 
     len = strlen(nativeTarget) * sizeof(WCHAR);
+    plen = strlen(nativePrint) * sizeof(WCHAR);
     reparseBuf->ReparseTag = IO_REPARSE_TAG_MOUNT_POINT;
-    reparseBuf->ReparseDataLength = len + 12;
+    reparseBuf->ReparseDataLength = len + plen + 12;
     reparseBuf->Reserved = 0;
     reparseBuf->SubstituteNameOffset = 0;
     reparseBuf->SubstituteNameLength = len;
     reparseBuf->PrintNameOffset = len + sizeof(WCHAR);
-    reparseBuf->PrintNameLength = 0;
+    reparseBuf->PrintNameLength = plen;
     MultiByteToWideChar(CP_ACP, 0, nativeTarget, -1,
                         reparseBuf->PathBuffer, MAX_PATH);
+    MultiByteToWideChar(CP_ACP, 0, nativePrint, -1,
+                        &reparseBuf->PathBuffer[reparseBuf->PrintNameOffset/2], MAX_PATH);
 
     /*
      * FSCTL_SET_REPARSE_POINT is coded differently depending on SDK version;
@@ -308,15 +336,27 @@ pgreadlink(const char *path, char *buf, size_t size)
     /* Got it, let's get some results from this */
     if (reparseBuf->ReparseTag != IO_REPARSE_TAG_MOUNT_POINT)
     {
-        errno = EINVAL;
-        return -1;
+      errno = EINVAL;
+      return -1;
     }
 
-    r = WideCharToMultiByte(CP_ACP, 0,
-                            reparseBuf->PathBuffer, -1,
-                            buf,
-                            size,
-                            NULL, NULL);
+    /*
+     * if PrintName is registered, this link is emulated relative
+     * symbolic link. Return the PrintName rather than SubstituteName.
+     */
+    if (reparseBuf->PrintNameLength > 0)
+      r = WideCharToMultiByte(CP_ACP, 0,
+                              &reparseBuf->PathBuffer[reparseBuf->PrintNameOffset / 2],
+                              -1,
+                              buf,
+                              size,
+                              NULL, NULL);
+    else
+      r = WideCharToMultiByte(CP_ACP, 0,
+                              reparseBuf->PathBuffer, -1,
+                              buf,
+                              size,
+                              NULL, NULL);
 
     if (r <= 0)
     {
@@ -328,8 +368,9 @@ pgreadlink(const char *path, char *buf, size_t size)
      * If the path starts with "\??\", which it will do in most (all?) cases,
      * strip those out.
      */
-    if (r > 4 && strncmp(buf, "\\??\\", 4) == 0)
-    {
+    if (reparseBuf->PrintNameLength == 0 &&
+        r > 4 && strncmp(buf, "\\??\\", 4) == 0)
+    {
         memmove(buf, buf + 4, strlen(buf + 4) + 1);
         r -= 4;
     }
-- 
2.16.3