On Mon, Jun 29, 2015 at 3:46 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> On 06/24/2015 09:43 AM, Michael Paquier wrote:
>>
>> Attached is a new set of patches. Except for the last ones that
>> addresses one issue of pg_rewind (symlink management when streaming
>> PGDATA), all the others introduce if_not_exists options for the
>> functions of genfile.c. The pg_rewind stuff could be more polished
>> though. Feel free to comment.
>
> I've committed the additional option to the functions in genfile.c (I
> renamed it to "missing_ok", for clarity), and the pg_rewind changes to use
> that option.
>
> I ended up refactoring the patch quite a bit, so if you could double-check
> what I committed to make sure I didn't break anything, that would be great.
Thanks, the new patch looks far better than what I did, I noticed a
couple of typos in the docs though:
- s/behaviour/behavior, "behavior" is American English spelling, and
it is the one used elsewhere as well, hence I guess that it makes
sense to our it.
- s/an non-existent/a non-existent
- pg_proc.h is still using if_not_exists as in my patch (you corrected
it to use missing_ok).
Those are fixed as 0001 in the attached set.
> I didn't commit the tablespace or symlink handling changes yet, will review
> those separately.
Thanks. Attached are rebased versions that take into account your
previous changes as well (like the variable renaming, wrapper function
usage, etc). I also added missing_ok to pg_readlink for consistency,
and rebased the fix of pg_rewind for soft links with 0005. Note that
0005 does not use pg_tablespace_location(), still the patch series
include an implementation of missing_ok for it. Feel free to use it if
necessary.
> I also didn't commit the new regression test yet. It would indeed be nice to
> have one, but I think it was a few bricks shy of a load. It should work in a
> freshly initdb'd system, but not necessarily on an existing installation.
> First, it relied on the fact that postgresql.conf.auto exists, but a DBA
> might remove that if he wants to make sure the feature is not used.
> Secondly, it relied on the fact that pg_twophase is empty, but there is no
> guarantee of that either. Third, the error messages included in the expected
> output, e.g "No such file or directory", depend on the operating system and
> locale. And finally, it'd be nice to test more things, in particular the
> behaviour of different offsets and lengths to pg_read_binary_file(),
> although an incomplete test would be better than no test at all.
Portability is going to really reduce the test range, the only things
that we could test are:
- NULL results returned when missing_ok = true (with a dummy file
name/link/directory) as missing_ok = false would choke depending on
the platform as you mentioned.
- Ensure that those functions called by users without superuser rights
fail properly.
- Path format errors for each function, like that:
=# select pg_ls_dir('..');
ERROR: 42501: path must be in or below the current directory
LOCATION: convert_and_check_filename, genfile.c:78
For tests on pg_read_binary_file, do you think that there is one file
of PGDATA that we could use for scanning? I cannot think of one on the
top of my mind now (postgresql.conf or any configuration files could
be overridden by the user so they are out of scope, PG_VERSION is an
additional maintenance burden). Well, I think that those things are
still worth testing in any case and I think that you think so as well.
If you are fine with that I could wrap up a patch that's better than
nothing done for sure. Thoughts?
Now we could have a TAP test for this stuff, where we could have a
custom PGDATA with some dummy files that we know will be empty for
example, still it seems like an overkill to me for this purpose,
initdb is rather costly in this facility.. And on small machines.
Regards,
--
Michael
From 378bd8af5c27e7e20b038f05b693b95e9871ef0b Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 29 Jun 2015 13:38:03 +0900
Subject: [PATCH 1/5] Fix some typos and an incorrect naming introduced by
cb2acb1
---
doc/src/sgml/func.sgml | 4 ++--
src/include/catalog/pg_proc.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d49cd43..8b28e29 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17851,7 +17851,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
<para>
All of these functions take an optional <parameter>missing_ok</> parameter,
- which specifies the behaviour when the file or directory does not exist.
+ which specifies the behavior when the file or directory does not exist.
If <literal>true</literal>, the function returns NULL (except
<function>pg_ls_dir</>, which returns an empty result set). If
<literal>false</>, an error is raised. The default is <literal>false</>.
@@ -17867,7 +17867,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
included in the result set. The default is to exclude them
(<literal>false</>), but including them can be useful when
<parameter>missing_ok</> is <literal>true</literal>, to distinguish an
- empty directory from an non-existent directory.
+ empty directory from a non-existent directory.
</para>
<indexterm>
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 3a40fa6..be3a8fb 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3183,7 +3183,7 @@ DESCR("rotate log file");
DATA(insert OID = 2623 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2249 "25" "{25,20,1184,1184,1184,1184,16}" "{i,o,o,o,o,o,o}" "{filename,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file_1arg _null_ _null_ _null_ ));
DESCR("get information about file");
-DATA(insert OID = 3307 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 2249 "25 16" "{25,16,20,1184,1184,1184,1184,16}" "{i,i,o,o,o,o,o,o}" "{filename,if_not_exists,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file _null_ _null_ _null_ ));
+DATA(insert OID = 3307 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 2249 "25 16" "{25,16,20,1184,1184,1184,1184,16}" "{i,i,o,o,o,o,o,o}" "{filename,missing_ok,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file _null_ _null_ _null_ ));
DESCR("get information about file");
DATA(insert OID = 2624 ( pg_read_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 3 0 25 "25 20 20" _null_ _null_ _null_ _null_ _null_ pg_read_file_off_len _null_ _null_ _null_ ));
DESCR("read text from a file");
--
2.4.5
From 9ae79930461a9024d01a878bdf63bfc4cd143c0d Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Wed, 24 Jun 2015 11:26:02 +0900
Subject: [PATCH 2/5] Add new column islink in pg_stat_file
This allows to detect if the file path evaluated is a symlink or not.
pg_stat_file is switched to use as well lstat to enable soft link
detection.
---
doc/src/sgml/func.sgml | 5 +++--
src/backend/utils/adt/genfile.c | 15 +++++++++++----
src/include/catalog/pg_proc.h | 5 ++---
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8b28e29..67e1626 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17905,8 +17905,9 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
<function>pg_stat_file</> returns a record containing the file
size, last accessed time stamp, last modified time stamp,
last file status change time stamp (Unix platforms only),
- file creation time stamp (Windows only), and a <type>boolean</type>
- indicating if it is a directory. Typical usages include:
+ file creation time stamp (Windows only), a <type>boolean</type>
+ indicating if it is a directory, and a <type>boolean</type>
+ indicating if it is a symbolic link. Typical usages include:
<programlisting>
SELECT * FROM pg_stat_file('filename');
SELECT (pg_stat_file('filename')).modification;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index c4eb10d..df8ae88 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -306,8 +306,8 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_P(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool isnull[6];
+ Datum values[7];
+ bool isnull[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
@@ -323,7 +323,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
filename = convert_and_check_filename(filename_t);
- if (stat(filename, &fst) < 0)
+ if (lstat(filename, &fst) < 0)
{
if (missing_ok && errno == ENOENT)
PG_RETURN_NULL();
@@ -337,7 +337,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6, false);
+ tupdesc = CreateTemplateTupleDesc(7, false);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -350,6 +350,8 @@ pg_stat_file(PG_FUNCTION_ARGS)
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
"isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
+ "islink", BOOLOID, -1, 0);
BlessTupleDesc(tupdesc);
memset(isnull, false, sizeof(isnull));
@@ -366,6 +368,11 @@ pg_stat_file(PG_FUNCTION_ARGS)
values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime));
#endif
values[5] = BoolGetDatum(S_ISDIR(fst.st_mode));
+#ifndef WIN32
+ values[6] = BoolGetDatum(S_ISLNK(fst.st_mode));
+#else
+ values[6] = BoolGetDatum(pgwin32_is_junction(filename));
+#endif
tuple = heap_form_tuple(tupdesc, values, isnull);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index be3a8fb..f6abd74 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3180,10 +3180,9 @@ DATA(insert OID = 2621 ( pg_reload_conf PGNSP PGUID 12 1 0 0 0 f f f f t f v 0
DESCR("reload configuration files");
DATA(insert OID = 2622 ( pg_rotate_logfile PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 16 "" _null_ _null_ _null_ _null_ _null_ pg_rotate_logfile _null_ _null_ _null_ ));
DESCR("rotate log file");
-
-DATA(insert OID = 2623 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2249 "25" "{25,20,1184,1184,1184,1184,16}" "{i,o,o,o,o,o,o}" "{filename,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file_1arg _null_ _null_ _null_ ));
+DATA(insert OID = 2623 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2249 "25" "{25,20,1184,1184,1184,1184,16,16}" "{i,o,o,o,o,o,o,o}" "{filename,size,access,modification,change,creation,isdir,islink}" _null_ _null_ pg_stat_file_1arg _null_ _null_ _null_ ));
DESCR("get information about file");
-DATA(insert OID = 3307 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 2249 "25 16" "{25,16,20,1184,1184,1184,1184,16}" "{i,i,o,o,o,o,o,o}" "{filename,missing_ok,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file _null_ _null_ _null_ ));
+DATA(insert OID = 3307 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 2249 "25 16" "{25,16,20,1184,1184,1184,1184,16,16}" "{i,i,o,o,o,o,o,o,o}" "{filename,missing_ok,size,access,modification,change,creation,isdir,islink}" _null_ _null_ pg_stat_file _null_ _null_ _null_ ));
DESCR("get information about file");
DATA(insert OID = 2624 ( pg_read_file PGNSP PGUID 12 1 0 0 0 f f f f t f v 3 0 25 "25 20 20" _null_ _null_ _null_ _null_ _null_ pg_read_file_off_len _null_ _null_ _null_ ));
DESCR("read text from a file");
--
2.4.5
From 0ea59fb774e61e60def14f5a3470429695ad6adc Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 29 Jun 2015 14:40:20 +0900
Subject: [PATCH 3/5] Add pg_readlink to get value of a symbolic link
This is similar to Posix's readlink, except that PostgreSQL restrictions
are applied to the source path queried, similarly to other functions of
genfile.c.
---
doc/src/sgml/func.sgml | 9 ++++++
src/backend/utils/adt/genfile.c | 66 +++++++++++++++++++++++++++++++++++++++++
src/include/catalog/pg_proc.h | 4 +++
src/include/utils/builtins.h | 2 ++
4 files changed, 81 insertions(+)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 67e1626..80a4f26 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17845,6 +17845,15 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
Return information about a file.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_readlink(<parameter>filename</> <type>text</>[, <parameter>missing_ok</> <type>boolean</type>])</function></literal>
+ </entry>
+ <entry><type>text</type></entry>
+ <entry>
+ Return value of a symbolic link.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index df8ae88..a0f8d7b 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -480,3 +480,69 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
{
return pg_ls_dir(fcinfo);
}
+
+
+/*
+ * pg_readlink - Find the real location of the provided symbolic link.
+ */
+Datum
+pg_readlink(PG_FUNCTION_ARGS)
+{
+ text *sourcepath_t = PG_GETARG_TEXT_P(0);
+ char *sourcepath;
+ char targetpath[MAXPGPATH];
+ int rllen;
+ bool missing_ok = false;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to read files"))));
+
+ if (PG_NARGS() == 2)
+ missing_ok = PG_GETARG_BOOL(1);
+
+#if defined(HAVE_READLINK) || defined(WIN32)
+ sourcepath = convert_and_check_filename(sourcepath_t);
+
+ rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
+ if (rllen < 0)
+ {
+ if (missing_ok && errno == ENOENT)
+ PG_RETURN_NULL();
+ else
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not read symbolic link \"%s\": %m",
+ sourcepath)));
+ }
+ if (rllen >= sizeof(targetpath))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("symbolic link \"%s\" target is too long",
+ sourcepath)));
+ }
+ targetpath[rllen] = '\0';
+
+ PG_RETURN_TEXT_P(cstring_to_text(targetpath));
+#else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("symbolic links are not supported by this platform")));
+ PG_RETURN_NULL();
+#endif
+}
+
+/*
+ * pg_readlink (1 argument version)
+ *
+ * note: this wrapper is necessary to pass the sanity check in opr_sanity,
+ * which checks that all built-in functions that share the implementing C
+ * function take the same number of arguments
+ */
+Datum
+pg_readlink_1arg(PG_FUNCTION_ARGS)
+{
+ return pg_readlink(fcinfo);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index f6abd74..f9c6176 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3200,6 +3200,10 @@ DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0
DESCR("list all files in a directory");
DATA(insert OID = 3297 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 3 0 25 "25 16 16" _null_ _null_ _null_ _null_ _null_ pg_ls_dir _null_ _null_ _null_ ));
DESCR("list all files in a directory");
+DATA(insert OID = 3298 ( pg_readlink PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 25 "25" _null_ _null_ _null_ _null_ _null_ pg_readlink_1arg _null_ _null_ _null_ ));
+DESCR("read value of a symbolic link");
+DATA(insert OID = 3299 ( pg_readlink PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 25 "25 16" _null_ _null_ _null_ _null_ _null_ pg_readlink _null_ _null_ _null_ ));
+DESCR("read value of a symbolic link");
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "701" _null_ _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ ));
DESCR("sleep for the specified time in seconds");
DATA(insert OID = 3935 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 "1186" _null_ _null_ _null_ _null_ _null_ "select pg_catalog.pg_sleep(extract(epoch from pg_catalog.clock_timestamp() operator(pg_catalog.+) $1) operator(pg_catalog.-) extract(epoch from pg_catalog.clock_timestamp()))" _null_ _null_ _null_ ));
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 9855672..615e5ab 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -479,6 +479,8 @@ extern Datum pg_read_binary_file_off_len(PG_FUNCTION_ARGS);
extern Datum pg_read_binary_file_all(PG_FUNCTION_ARGS);
extern Datum pg_ls_dir(PG_FUNCTION_ARGS);
extern Datum pg_ls_dir_1arg(PG_FUNCTION_ARGS);
+extern Datum pg_readlink(PG_FUNCTION_ARGS);
+extern Datum pg_readlink_1arg(PG_FUNCTION_ARGS);
/* misc.c */
extern Datum current_database(PG_FUNCTION_ARGS);
--
2.4.5
From bdf72b13775b7fa71d24a92c71fc0c4002109e01 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 29 Jun 2015 15:07:24 +0900
Subject: [PATCH 4/5] Extend pg_tablespace_location with option missing_ok
This is useful for code paths that prefer receive an empty response
instead of an ERROR if tablespace specified does not exist.
---
doc/src/sgml/func.sgml | 9 +++++++--
src/backend/utils/adt/misc.c | 30 ++++++++++++++++++++++++++----
src/include/catalog/pg_proc.h | 2 ++
src/include/utils/builtins.h | 1 +
4 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 80a4f26..0257df6 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15850,9 +15850,14 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
<entry>get the set of database OIDs that have objects in the tablespace</entry>
</row>
<row>
- <entry><literal><function>pg_tablespace_location(<parameter>tablespace_oid</parameter>)</function></literal></entry>
+ <entry><literal><function>pg_tablespace_location(<parameter>tablespace_oid</parameter> [, <parameter>missing_ok</> <type>boolean</type>])</function></literal></entry>
<entry><type>text</type></entry>
- <entry>get the path in the file system that this tablespace is located in</entry>
+ <entry>
+ Get the path in the file system that this tablespace is located in.
+ If <parameter>missing_ok</> is <literal>true</>, the function returns
+ NULL when the tablespace does not exist. if <literal>false</>, an
+ error is raised. The default is <literal>false</>.
+ </entry>
</row>
<row>
<entry><literal><function>pg_typeof(<parameter>any</parameter>)</function></literal></entry>
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index c0495d9..f5f61c5 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -347,6 +347,10 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
char sourcepath[MAXPGPATH];
char targetpath[MAXPGPATH];
int rllen;
+ bool missing_ok = false;
+
+ if (PG_NARGS() == 2)
+ missing_ok = PG_GETARG_BOOL(1);
/*
* It's useful to apply this function to pg_class.reltablespace, wherein
@@ -373,10 +377,15 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
if (rllen < 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not read symbolic link \"%s\": %m",
- sourcepath)));
+ {
+ if (missing_ok && errno == ENOENT)
+ PG_RETURN_NULL();
+ else
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not read symbolic link \"%s\": %m",
+ sourcepath)));
+ }
if (rllen >= sizeof(targetpath))
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
@@ -394,6 +403,19 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
}
/*
+ * pg_tablespace_location (1 argument version)
+ *
+ * note: this wrapper is necessary to pass the sanity check in opr_sanity,
+ * which checks that all built-in functions that share the implementing C
+ * function take the same number of arguments.
+ */
+Datum
+pg_tablespace_location_1arg(PG_FUNCTION_ARGS)
+{
+ return pg_tablespace_location(fcinfo);
+}
+
+/*
* pg_sleep - delay for N seconds
*/
Datum
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index f9c6176..6ccbf9b 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2922,6 +2922,8 @@ DESCR("current trigger depth");
DATA(insert OID = 3778 ( pg_tablespace_location PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 25 "26" _null_ _null_ _null_ _null_ _null_ pg_tablespace_location _null_ _null_ _null_ ));
DESCR("tablespace location");
+DATA(insert OID = 3579 ( pg_tablespace_location PGNSP PGUID 12 1 0 0 0 f f f f t f s 2 0 25 "26 16" _null_ _null_ _null_ _null_ _null_ pg_tablespace_location_1arg _null_ _null_ _null_ ));
+DESCR("tablespace location");
DATA(insert OID = 1946 ( encode PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 25 "17 25" _null_ _null_ _null_ _null_ _null_ binary_encode _null_ _null_ _null_ ));
DESCR("convert bytea value into some ascii-only text string");
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 615e5ab..150afb6 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -490,6 +490,7 @@ extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
extern Datum pg_tablespace_location(PG_FUNCTION_ARGS);
+extern Datum pg_tablespace_location_1arg(PG_FUNCTION_ARGS);
extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);
extern Datum pg_sleep(PG_FUNCTION_ARGS);
extern Datum pg_get_keywords(PG_FUNCTION_ARGS);
--
2.4.5
From f44c286ec129deaf120986a5025996001cfd8ffe Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 29 Jun 2015 15:09:34 +0900
Subject: [PATCH 5/5] Fix symlink usage in pg_rewind
Make use of pg_readlink to determine where a symlink in a source PGDATA
streamed is from. Also remove restrictions related to symlink comparisons
on source and target instances, those need to be only limited to
tablespaces.
---
src/bin/pg_rewind/file_ops.c | 2 ++
src/bin/pg_rewind/filemap.c | 11 +++--------
src/bin/pg_rewind/filemap.h | 1 +
src/bin/pg_rewind/libpq_fetch.c | 32 ++++++++++++++++----------------
4 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index c2d8aa1..29ddb1d 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -139,6 +139,7 @@ remove_target(file_entry_t *entry)
remove_target_file(entry->path);
break;
+ case FILE_TYPE_TABLESPACE:
case FILE_TYPE_SYMLINK:
remove_target_symlink(entry->path);
break;
@@ -156,6 +157,7 @@ create_target(file_entry_t *entry)
create_target_dir(entry->path);
break;
+ case FILE_TYPE_TABLESPACE:
case FILE_TYPE_SYMLINK:
create_target_symlink(entry->path, entry->link_target);
break;
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 05eff68..6ef2342 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -112,12 +112,6 @@ process_source_file(const char *path, file_type_t type, size_t newsize,
switch (type)
{
case FILE_TYPE_DIRECTORY:
- if (exists && !S_ISDIR(statbuf.st_mode))
- {
- /* it's a directory in source, but not in target. Strange.. */
- pg_fatal("\"%s\" is not a directory\n", localpath);
- }
-
if (!exists)
action = FILE_ACTION_CREATE;
else
@@ -125,7 +119,7 @@ process_source_file(const char *path, file_type_t type, size_t newsize,
oldsize = 0;
break;
- case FILE_TYPE_SYMLINK:
+ case FILE_TYPE_TABLESPACE:
if (exists &&
#ifndef WIN32
!S_ISLNK(statbuf.st_mode)
@@ -140,7 +134,8 @@ process_source_file(const char *path, file_type_t type, size_t newsize,
*/
pg_fatal("\"%s\" is not a symbolic link\n", localpath);
}
-
+ /* fallback to default */
+ case FILE_TYPE_SYMLINK:
if (!exists)
action = FILE_ACTION_CREATE;
else
diff --git a/src/bin/pg_rewind/filemap.h b/src/bin/pg_rewind/filemap.h
index 9943ec5..35540d6 100644
--- a/src/bin/pg_rewind/filemap.h
+++ b/src/bin/pg_rewind/filemap.h
@@ -36,6 +36,7 @@ typedef enum
{
FILE_TYPE_REGULAR,
FILE_TYPE_DIRECTORY,
+ FILE_TYPE_TABLESPACE,
FILE_TYPE_SYMLINK
} file_type_t;
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 05aa133..d77c931 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -140,30 +140,26 @@ libpqProcessFileList(void)
* Create a recursive directory listing of the whole data directory.
*
* The WITH RECURSIVE part does most of the work. The second part gets the
- * targets of the symlinks in pg_tblspc directory.
- *
- * XXX: There is no backend function to get a symbolic link's target in
- * general, so if the admin has put any custom symbolic links in the data
- * directory, they won't be copied correctly.
+ * targets of the symlinks in the data directory.
*/
sql =
- "WITH RECURSIVE files (path, filename, size, isdir) AS (\n"
- " SELECT '' AS path, filename, size, isdir FROM\n"
+ "WITH RECURSIVE files (path, filename, size, isdir, islink) AS (\n"
+ " SELECT '' AS path, filename, size, isdir, islink FROM\n"
" (SELECT pg_ls_dir('.', true, false) AS filename) AS fn,\n"
" pg_stat_file(fn.filename, true) AS this\n"
" UNION ALL\n"
" SELECT parent.path || parent.filename || '/' AS path,\n"
- " fn, this.size, this.isdir\n"
+ " fn, this.size, this.isdir, this.islink\n"
" FROM files AS parent,\n"
" pg_ls_dir(parent.path || parent.filename, true, false) AS fn,\n"
" pg_stat_file(parent.path || parent.filename || '/' || fn, true) AS this\n"
" WHERE parent.isdir = 't'\n"
")\n"
- "SELECT path || filename, size, isdir,\n"
- " pg_tablespace_location(pg_tablespace.oid) AS link_target\n"
- "FROM files\n"
- "LEFT OUTER JOIN pg_tablespace ON files.path = 'pg_tblspc/'\n"
- " AND oid::text = files.filename\n";
+ "SELECT path || filename, size, isdir, islink,\n"
+ " path = 'pg_tblspc/' AS istblspc,\n"
+ " CASE WHEN islink THEN pg_readlink(path || filename)\n"
+ " ELSE NULL END AS link_target\n"
+ "FROM files\n";
res = PQexec(conn, sql);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
@@ -171,7 +167,7 @@ libpqProcessFileList(void)
PQresultErrorMessage(res));
/* sanity check the result set */
- if (PQnfields(res) != 4)
+ if (PQnfields(res) != 6)
pg_fatal("unexpected result set while fetching file list\n");
/* Read result to local variables */
@@ -180,7 +176,9 @@ libpqProcessFileList(void)
char *path = PQgetvalue(res, i, 0);
int filesize = atoi(PQgetvalue(res, i, 1));
bool isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
- char *link_target = PQgetvalue(res, i, 3);
+ bool islink = (strcmp(PQgetvalue(res, i, 3), "t") == 0);
+ bool istblspc = (strcmp(PQgetvalue(res, i, 4), "t") == 0);
+ char *link_target = PQgetvalue(res, i, 5);
file_type_t type;
if (PQgetisnull(res, 0, 1))
@@ -192,7 +190,9 @@ libpqProcessFileList(void)
continue;
}
- if (link_target[0])
+ if (istblspc)
+ type = FILE_TYPE_TABLESPACE;
+ else if (islink)
type = FILE_TYPE_SYMLINK;
else if (isdir)
type = FILE_TYPE_DIRECTORY;
--
2.4.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
