Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)
Hi all,
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything. The issue has been discussed here
(/messages/by-id/54935D9D.7010608@dunslane.net)
but nobody actually sent a patch, so here is one, and a thread for
this discussion.
Regards,
--
Michael
Attachments:
20150115_pg_regress_create_destdir.patchtext/x-patch; charset=US-ASCII; name=20150115_pg_regress_create_destdir.patchDownload
*** a/src/test/regress/pg_regress.c
--- b/src/test/regress/pg_regress.c
***************
*** 496,501 **** convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
--- 496,502 ----
{
char testtablespace[MAXPGPATH];
char indir[MAXPGPATH];
+ char result_dir[MAXPGPATH];
struct stat st;
int ret;
char **name;
***************
*** 520,525 **** convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
--- 521,534 ----
/* Error logged in pgfnames */
exit(2);
+ /*
+ * Enforce creation of destination directory if it does not exist yet.
+ * This is useful for tests using only source files.
+ */
+ snprintf(result_dir, MAXPGPATH, "%s/%s", dest_dir, dest_subdir);
+ if (!directory_exists(result_dir))
+ make_directory(result_dir);
+
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
#ifdef WIN32
***************
*** 565,571 **** convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
/* build the full actual paths to open */
snprintf(prefix, strlen(*name) - 6, "%s", *name);
snprintf(srcfile, MAXPGPATH, "%s/%s", indir, *name);
! snprintf(destfile, MAXPGPATH, "%s/%s/%s.%s", dest_dir, dest_subdir,
prefix, suffix);
infile = fopen(srcfile, "r");
--- 574,580 ----
/* build the full actual paths to open */
snprintf(prefix, strlen(*name) - 6, "%s", *name);
snprintf(srcfile, MAXPGPATH, "%s/%s", indir, *name);
! snprintf(destfile, MAXPGPATH, "%s/%s.%s", result_dir,
prefix, suffix);
infile = fopen(srcfile, "r");
On 1/14/15 11:31 PM, Michael Paquier wrote:
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything.
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 20, 2015 at 5:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 1/14/15 11:31 PM, Michael Paquier wrote:
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything.We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?
I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/20/15 1:56 AM, Michael Paquier wrote:
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.
The affected modules have sql/.gitignore and/or expected/.gitignore
files, so the case that the directory doesn't exist and needs to be
created doesn't actually happen.
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 21, 2015 at 6:51 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/20/15 1:56 AM, Michael Paquier wrote:
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.The affected modules have sql/.gitignore and/or expected/.gitignore
files, so the case that the directory doesn't exist and needs to be
created doesn't actually happen.
What about extensions that do not have sql/ and extended/ in their
code tree? I don't have an example of such extensions in my mind but I
cannot assume either that they do not exist. See for example something
that happended a couple of months back, dummy_seclabel has been
trapped by that with df761e3, fixed afterwards with 3325624 (the
structure has been changed again since, but that's the error showed up
because of those missing sql/ and expected/).
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/22/15 5:41 AM, Michael Paquier wrote:
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 23, 2015 at 12:00 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/22/15 5:41 AM, Michael Paquier wrote:
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..
--
Michael
Attachments:
0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchtext/x-diff; charset=US-ASCII; name=0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchDownload
From 0b89d35f9605f866b45943d842898e30923476c3 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Mon, 23 Feb 2015 15:02:14 +0900
Subject: [PATCH 1/2] Enforce creation of input and output paths in pg_regress
This is particularly useful for extensions that have only regression tests
in input/ and output/ dynamically generated when running the tests to keep
the code tree of such extensions clean without empty folders containing as
only content a .gitignore ignoring everything else other than its own
existence.
---
src/test/regress/pg_regress.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 3af0e57..a7aa580 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -496,6 +496,7 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
{
char testtablespace[MAXPGPATH];
char indir[MAXPGPATH];
+ char result_dir[MAXPGPATH];
struct stat st;
int ret;
char **name;
@@ -520,6 +521,14 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
/* Error logged in pgfnames */
exit(2);
+ /*
+ * Enforce creation of destination directory if it does not exist yet.
+ * This is useful for tests using only source files.
+ */
+ snprintf(result_dir, MAXPGPATH, "%s/%s", dest_dir, dest_subdir);
+ if (!directory_exists(result_dir))
+ make_directory(result_dir);
+
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
#ifdef WIN32
@@ -565,7 +574,7 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
/* build the full actual paths to open */
snprintf(prefix, strlen(*name) - 6, "%s", *name);
snprintf(srcfile, MAXPGPATH, "%s/%s", indir, *name);
- snprintf(destfile, MAXPGPATH, "%s/%s/%s.%s", dest_dir, dest_subdir,
+ snprintf(destfile, MAXPGPATH, "%s/%s.%s", result_dir,
prefix, suffix);
infile = fopen(srcfile, "r");
--
2.3.0
0002-Add-regress_dynamic-as-a-test-module.patchtext/x-diff; charset=US-ASCII; name=0002-Add-regress_dynamic-as-a-test-module.patchDownload
From c5e58c85e743c3c7b133234588e2010612166f8f Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Mon, 23 Feb 2015 15:23:44 +0900
Subject: [PATCH 2/2] Add regress_dynamic as a test module
This simple extension has the characteristic to only contain non-static
regression test content, and is aimed to test if pg_regress is able to
generate appropriately input and output directories when they do not
exist.
---
src/test/modules/regress_dynamic/.gitignore | 8 ++++++++
src/test/modules/regress_dynamic/Makefile | 16 ++++++++++++++++
src/test/modules/regress_dynamic/README | 6 ++++++
src/test/modules/regress_dynamic/input/basic.source | 9 +++++++++
src/test/modules/regress_dynamic/output/basic.source | 11 +++++++++++
.../modules/regress_dynamic/regress_dynamic--1.0.sql | 8 ++++++++
src/test/modules/regress_dynamic/regress_dynamic.control | 5 +++++
7 files changed, 63 insertions(+)
create mode 100644 src/test/modules/regress_dynamic/.gitignore
create mode 100644 src/test/modules/regress_dynamic/Makefile
create mode 100644 src/test/modules/regress_dynamic/README
create mode 100644 src/test/modules/regress_dynamic/input/basic.source
create mode 100644 src/test/modules/regress_dynamic/output/basic.source
create mode 100644 src/test/modules/regress_dynamic/regress_dynamic--1.0.sql
create mode 100644 src/test/modules/regress_dynamic/regress_dynamic.control
diff --git a/src/test/modules/regress_dynamic/.gitignore b/src/test/modules/regress_dynamic/.gitignore
new file mode 100644
index 0000000..122ede3
--- /dev/null
+++ b/src/test/modules/regress_dynamic/.gitignore
@@ -0,0 +1,8 @@
+# Generated sub-directories
+/log/
+/results/
+/tmp_check/
+
+# Input and output directories of regression tests
+/expected/
+/sql/
diff --git a/src/test/modules/regress_dynamic/Makefile b/src/test/modules/regress_dynamic/Makefile
new file mode 100644
index 0000000..2cab345
--- /dev/null
+++ b/src/test/modules/regress_dynamic/Makefile
@@ -0,0 +1,16 @@
+EXTENSION = regress_dynamic
+DATA = regress_dynamic--1.0.sql
+PGFILEDESC = "regress_dynamic - extension with only non-static regression tests"
+
+REGRESS = basic
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/regress_dynamic
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/regress_dynamic/README b/src/test/modules/regress_dynamic/README
new file mode 100644
index 0000000..b039e5e
--- /dev/null
+++ b/src/test/modules/regress_dynamic/README
@@ -0,0 +1,6 @@
+regress_dynamic
+===============
+
+regress_dynamic is an extension used to test that pg_regress correctly
+creates input and output paths when regression test suite of an extension
+only contain non-static content with empty sql/ and expected/ folders.
diff --git a/src/test/modules/regress_dynamic/input/basic.source b/src/test/modules/regress_dynamic/input/basic.source
new file mode 100644
index 0000000..1d7031d
--- /dev/null
+++ b/src/test/modules/regress_dynamic/input/basic.source
@@ -0,0 +1,9 @@
+--
+-- regress_dynamic
+--
+
+-- Initialization
+CREATE EXTENSION regress_dynamic;
+
+-- Simple test
+SELECT * FROM dummy_tab;
diff --git a/src/test/modules/regress_dynamic/output/basic.source b/src/test/modules/regress_dynamic/output/basic.source
new file mode 100644
index 0000000..f666627
--- /dev/null
+++ b/src/test/modules/regress_dynamic/output/basic.source
@@ -0,0 +1,11 @@
+--
+-- regress_dynamic
+--
+-- Initialization
+CREATE EXTENSION regress_dynamic;
+-- Simple test
+SELECT * FROM dummy_tab;
+ id
+----
+(0 rows)
+
diff --git a/src/test/modules/regress_dynamic/regress_dynamic--1.0.sql b/src/test/modules/regress_dynamic/regress_dynamic--1.0.sql
new file mode 100644
index 0000000..c7a19c9
--- /dev/null
+++ b/src/test/modules/regress_dynamic/regress_dynamic--1.0.sql
@@ -0,0 +1,8 @@
+/* regress_dynamic/regress_dynamic--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION regress_dynamic" to load this file. \quit
+
+CREATE TABLE dummy_tab (
+ id int
+);
diff --git a/src/test/modules/regress_dynamic/regress_dynamic.control b/src/test/modules/regress_dynamic/regress_dynamic.control
new file mode 100644
index 0000000..643b946
--- /dev/null
+++ b/src/test/modules/regress_dynamic/regress_dynamic.control
@@ -0,0 +1,5 @@
+# regress_dynamic extension
+comment = 'regress_dynamic - extension with only non-static regression tests'
+default_version = '1.0'
+module_pathname = '$libdir/regress_dynamic'
+relocatable = true
--
2.3.0
On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
I don't see this as an improvement. What's wrong with a .gitignore file ignoring everything?
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 23, 2015 at 9:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I don't see this as an improvement. What's wrong with a .gitignore file
ignoring everything?
That's mainly a matter of code maintenance style (and I am on the side that
a minimum number of folders in a git tree makes things easier to follow),
and IMO it is strange to not have some flexibility. Btw, the reason why
this patch exists is this thread of last December:
/messages/by-id/20141212134508.GT1768@alvh.no-ip.org
--
Michael
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..
I was more thinking like we could use an existing module like file_fdw.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/24/15 1:23 AM, Michael Paquier wrote:
On Mon, Feb 23, 2015 at 9:31 PM, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com <mailto:michael.paquier@gmail.com>> wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I don't see this as an improvement. What's wrong with a .gitignore
file ignoring everything?That's mainly a matter of code maintenance style (and I am on the side
that a minimum number of folders in a git tree makes things easier to
follow), and IMO it is strange to not have some flexibility. Btw, the
reason why this patch exists is this thread of last December:
/messages/by-id/20141212134508.GT1768@alvh.no-ip.org
What's an example of this? I ask because what I'd like is to break the
cluster management portion of pg_regress out on it's own to make it easy
to get that functionality while using a different test framework (like
pgTap).
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 25, 2015 at 4:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..I was more thinking like we could use an existing module like file_fdw.
Right. I forgot this one. A patch is attached to do so.
--
Michael
Attachments:
0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchtext/x-diff; charset=US-ASCII; name=0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchDownload
From 65ccaf61a4c298a79747ec39a8dd75ec998a1125 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Mon, 23 Feb 2015 15:02:14 +0900
Subject: [PATCH 1/2] Enforce creation of input and output paths in pg_regress
This is particularly useful for extensions that have only regression tests
in input/ and output/ dynamically generated when running the tests to keep
the code tree of such extensions clean without empty folders containing as
only content a .gitignore ignoring everything else other than its own
existence.
---
src/test/regress/pg_regress.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 3af0e57..a7aa580 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -496,6 +496,7 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
{
char testtablespace[MAXPGPATH];
char indir[MAXPGPATH];
+ char result_dir[MAXPGPATH];
struct stat st;
int ret;
char **name;
@@ -520,6 +521,14 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
/* Error logged in pgfnames */
exit(2);
+ /*
+ * Enforce creation of destination directory if it does not exist yet.
+ * This is useful for tests using only source files.
+ */
+ snprintf(result_dir, MAXPGPATH, "%s/%s", dest_dir, dest_subdir);
+ if (!directory_exists(result_dir))
+ make_directory(result_dir);
+
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
#ifdef WIN32
@@ -565,7 +574,7 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c
/* build the full actual paths to open */
snprintf(prefix, strlen(*name) - 6, "%s", *name);
snprintf(srcfile, MAXPGPATH, "%s/%s", indir, *name);
- snprintf(destfile, MAXPGPATH, "%s/%s/%s.%s", dest_dir, dest_subdir,
+ snprintf(destfile, MAXPGPATH, "%s/%s.%s", result_dir,
prefix, suffix);
infile = fopen(srcfile, "r");
--
2.3.0
0002-Remove-expected-and-sql-in-file_fdw.patchtext/x-diff; charset=US-ASCII; name=0002-Remove-expected-and-sql-in-file_fdw.patchDownload
From 7f290ab2a4ffb6b59bdd836d5d61167f31a4845e Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Thu, 26 Feb 2015 14:26:01 +0900
Subject: [PATCH 2/2] Remove expected/ and sql/ in file_fdw
pg_regress is in charge of creating those folders if they do not exist.
---
contrib/file_fdw/.gitignore | 2 ++
contrib/file_fdw/expected/.gitignore | 1 -
contrib/file_fdw/sql/.gitignore | 1 -
3 files changed, 2 insertions(+), 2 deletions(-)
delete mode 100644 contrib/file_fdw/expected/.gitignore
delete mode 100644 contrib/file_fdw/sql/.gitignore
diff --git a/contrib/file_fdw/.gitignore b/contrib/file_fdw/.gitignore
index 5dcb3ff..9834c6d 100644
--- a/contrib/file_fdw/.gitignore
+++ b/contrib/file_fdw/.gitignore
@@ -1,4 +1,6 @@
# Generated subdirectories
+/expected/
/log/
/results/
+/sql/
/tmp_check/
diff --git a/contrib/file_fdw/expected/.gitignore b/contrib/file_fdw/expected/.gitignore
deleted file mode 100644
index a464ad1..0000000
--- a/contrib/file_fdw/expected/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-/file_fdw.out
diff --git a/contrib/file_fdw/sql/.gitignore b/contrib/file_fdw/sql/.gitignore
deleted file mode 100644
index ebf16fe..0000000
--- a/contrib/file_fdw/sql/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-/file_fdw.sql
--
2.3.0
* Michael Paquier (michael.paquier@gmail.com) wrote:
On Wed, Feb 25, 2015 at 4:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..I was more thinking like we could use an existing module like file_fdw.
Right. I forgot this one. A patch is attached to do so.
I'm always doing VPATH builds, but I believe there's still an
outstanding question on this from Peter about if we need to add entries
to .gitignore for, eg, sql/file_fdw.sql if these directories don't exist
because otherwise those would show up to someone doing a git status and
to Robert's point- if we need those anyway, it's probably better to have
them be in those directories rather than up a level and referring to
directories that don't exist in the tree.
Having the extra directories already exist doesn't seem like a bad thing
to me anyway. If they are causing confusion or something then perhaps a
README could be added to let people know why they exist.
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.
Thanks!
Stephen
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.
Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Michael Paquier (michael.paquier@gmail.com) wrote:
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
Works for me.
Thanks!
Stephen
Michael Paquier wrote:
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
Well I also don't really like the existing behavior but it's not a big
issue and so I'm OK with letting go.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers