postgresql.auto.conf read from wrong directory
Hi,
if you split configuration and data by setting data_directory,
postgresql.auto.conf is writen to the data directory
(/var/lib/postgresql/9.4/main in Debian), but tried to be read from
the etc directory (/etc/postgresql/9.4/main).
One place to fix it would be in ProcessConfigFile in
src/backend/utils/misc/guc-file.l:162 by always setting
CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
that breaks later in AbsoluteConfigLocation() when data_directory is
NULL. (As the comment in ProcessConfigFile says.)
I'm not sure where to break that dependency loop - the fix itself
seems easy, just don't tell to look for postgresql.auto.conf next to
ConfigFileName, but in the data_directory.
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 6, 2014 at 7:04 PM, Christoph Berg <cb@df7cb.de> wrote:
Hi,
if you split configuration and data by setting data_directory,
postgresql.auto.conf is writen to the data directory
(/var/lib/postgresql/9.4/main in Debian), but tried to be read from
the etc directory (/etc/postgresql/9.4/main).One place to fix it would be in ProcessConfigFile in
src/backend/utils/misc/guc-file.l:162 by always setting
CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
that breaks later in AbsoluteConfigLocation() when data_directory is
NULL. (As the comment in ProcessConfigFile says.)I'm not sure where to break that dependency loop - the fix itself
seems easy, just don't tell to look for postgresql.auto.conf next to
ConfigFileName, but in the data_directory.
Thanks for the report, will look into it.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.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 Tue, May 6, 2014 at 7:04 PM, Christoph Berg <cb@df7cb.de> wrote:
Hi,
if you split configuration and data by setting data_directory,
postgresql.auto.conf is writen to the data directory
(/var/lib/postgresql/9.4/main in Debian), but tried to be read from
the etc directory (/etc/postgresql/9.4/main).One place to fix it would be in ProcessConfigFile in
src/backend/utils/misc/guc-file.l:162 by always setting
CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
that breaks later in AbsoluteConfigLocation() when data_directory is
NULL. (As the comment in ProcessConfigFile says.)
This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.
Another way to fix could be that during parse, we directly set the config
value of data_directory, but I didn't prefer that way because the parse
routine is called from multiple paths which later on process the values
separately.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
fix_processing_auto_conf-v1.patchapplication/octet-stream; name=fix_processing_auto_conf-v1.patchDownload
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index ee38040..d5966a9 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -44,6 +44,7 @@ enum {
static unsigned int ConfigFileLineno;
static const char *GUC_flex_fatal_errmsg;
static sigjmp_buf *GUC_flex_fatal_jmp;
+static char *tmp_data_directory;
/* flex fails to supply a prototype for yylex, so provide one */
int GUC_yylex(void);
@@ -122,6 +123,8 @@ ProcessConfigFile(GucContext context)
int i;
char *ErrorConfFile;
char *CallingFileName;
+ char AutoConfFileName[MAXPGPATH];
+ char *datadir;
/*
* Config files are processed on startup (by the postmaster only)
@@ -155,11 +158,28 @@ ProcessConfigFile(GucContext context)
* set till this point, so use ConfigFile path which will be same.
*/
if (data_directory)
+ {
+ snprintf(AutoConfFileName,sizeof(AutoConfFileName),"%s",
+ PG_AUTOCONF_FILENAME);
CallingFileName = NULL;
+ }
+ else if (tmp_data_directory)
+ {
+ datadir = make_absolute_path(tmp_data_directory);
+ join_path_components(AutoConfFileName, datadir,
+ PG_AUTOCONF_FILENAME);
+ canonicalize_path(AutoConfFileName);
+ free(datadir);
+ CallingFileName = NULL;
+ }
else
+ {
+ snprintf(AutoConfFileName,sizeof(AutoConfFileName),"%s",
+ PG_AUTOCONF_FILENAME);
CallingFileName = ConfigFileName;
+ }
- if (!ParseConfigFile(PG_AUTOCONF_FILENAME, CallingFileName, false, 0, elevel, &head, &tail))
+ if (!ParseConfigFile(AutoConfFileName, CallingFileName, false, 0, elevel, &head, &tail))
{
/* Syntax error(s) detected in the file, so bail out */
error = true;
@@ -654,6 +674,15 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
}
else
{
+ /*
+ * In case of postmaster start for processing PG_AUTOCONF_FILENAME,
+ * we need to know the value of data_directory which is available
+ * only after processing of configuration files. Store it's value
+ * for use till data_directory is set.
+ */
+ if (guc_name_compare(opt_name, "data_directory") == 0)
+ tmp_data_directory = opt_value;
+
/* ordinary variable, append to list */
item = palloc(sizeof *item);
item->name = opt_name;
Re: Amit Kapila 2014-05-07 <CAA4eK1KTJkpVMnkOS2gFnnh2ZskO4ggDsPSWsHJbq1Cpu9EWRQ@mail.gmail.com>
This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.
Hi Amit,
the patch fixes the problem. Thanks for looking into this.
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
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, May 7, 2014 at 4:57 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, May 6, 2014 at 7:04 PM, Christoph Berg <cb@df7cb.de> wrote:
Hi,
if you split configuration and data by setting data_directory,
postgresql.auto.conf is writen to the data directory
(/var/lib/postgresql/9.4/main in Debian), but tried to be read from
the etc directory (/etc/postgresql/9.4/main).One place to fix it would be in ProcessConfigFile in
src/backend/utils/misc/guc-file.l:162 by always setting
CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
that breaks later in AbsoluteConfigLocation() when data_directory is
NULL. (As the comment in ProcessConfigFile says.)This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.
Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, May 8, 2014 at 6:51 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, May 7, 2014 at 4:57 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now.
Yes, that will also be issue similar to above.
Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?
Another way could be that we detect the same during server start
(while parsing postgresql.auto.conf) and then allow for reparse of
auto conf file, but I think that will be bit more complicated. So lets
try to solve it in a way suggested by you. If there is no objection by
anyone else then I will update the patch.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Amit Kapila (amit.kapila16@gmail.com) wrote:
On Thu, May 8, 2014 at 6:51 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?Another way could be that we detect the same during server start
(while parsing postgresql.auto.conf) and then allow for reparse of
auto conf file, but I think that will be bit more complicated. So lets
try to solve it in a way suggested by you. If there is no objection by
anyone else then I will update the patch.
Pretty sure this is more-or-less exactly what I suggested quite a while
back during the massive ALTER SYSTEM SET thread.. There are certain
options which we shouldn't be allowing in .auto.conf. I doubt this is
going to be the only one...
Thanks,
Stephen
On 2014-05-08 22:21:43 +0900, Fujii Masao wrote:
On Wed, May 7, 2014 at 4:57 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, May 6, 2014 at 7:04 PM, Christoph Berg <cb@df7cb.de> wrote:
Hi,
if you split configuration and data by setting data_directory,
postgresql.auto.conf is writen to the data directory
(/var/lib/postgresql/9.4/main in Debian), but tried to be read from
the etc directory (/etc/postgresql/9.4/main).One place to fix it would be in ProcessConfigFile in
src/backend/utils/misc/guc-file.l:162 by always setting
CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
that breaks later in AbsoluteConfigLocation() when data_directory is
NULL. (As the comment in ProcessConfigFile says.)This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?
I think that's a case of "Doctor, it hurts when I do this. Doctor: don't
do that then".
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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
Re: Andres Freund 2014-05-08 <20140508145901.GB1703@awork2.anarazel.de>
Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?I think that's a case of "Doctor, it hurts when I do this. Doctor: don't
do that then".
I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
other options, I agree with Andres that you should get to keep all
parts if you manage to break it.
Fortunately, ALTER SYSTEM already refuses to change config_file :)
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, May 8, 2014 at 9:47 PM, Christoph Berg <cb@df7cb.de> wrote:
Re: Andres Freund 2014-05-08 <20140508145901.GB1703@awork2.anarazel.de>
Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?I think that's a case of "Doctor, it hurts when I do this. Doctor: don't
do that then".I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
other options, I agree with Andres that you should get to keep all
parts if you manage to break it.
There is no harm in forbidding data_directory, but similarly we can
imagine that people can set some very large values for some config
variables due to which later it can have symptoms similar to this
issue.
Fortunately, ALTER SYSTEM already refuses to change config_file :)
That is because config_file is not a file parameter (not present in
postgresql.conf). We disallow non-file and internal (like wal_segment_size)
parameters in Alter System
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.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 Fri, May 9, 2014 at 1:06 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, May 8, 2014 at 9:47 PM, Christoph Berg <cb@df7cb.de> wrote:
Re: Andres Freund 2014-05-08 <20140508145901.GB1703@awork2.anarazel.de>
Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?I think that's a case of "Doctor, it hurts when I do this. Doctor: don't
do that then".I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
other options, I agree with Andres that you should get to keep all
parts if you manage to break it.There is no harm in forbidding data_directory, but similarly we can
imagine that people can set some very large values for some config
variables due to which later it can have symptoms similar to this
issue.
Yes, that can prevent the server from restarting at all. In this case,
to restart the server, we need to edit postgresql.auto.conf manually
and remove the problematic settings though the header of
postgresql.auto.conf warns "Do not edit this file manually!".
Regards,
--
Fujii Masao
--
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, May 9, 2014 at 7:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, May 9, 2014 at 1:06 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
There is no harm in forbidding data_directory, but similarly we can
imagine that people can set some very large values for some config
variables due to which later it can have symptoms similar to this
issue.Yes, that can prevent the server from restarting at all. In this case,
to restart the server, we need to edit postgresql.auto.conf manually
and remove the problematic settings though the header of
postgresql.auto.conf warns "Do not edit this file manually!".
So lets not try to forbid data_directory in postgresql.auto.conf and just
fix the case reported in this mail for postgresql.conf for which the
patch is attached upthread.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.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 Sat, May 10, 2014 at 12:30 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Fri, May 9, 2014 at 7:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, May 9, 2014 at 1:06 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
There is no harm in forbidding data_directory, but similarly we can
imagine that people can set some very large values for some config
variables due to which later it can have symptoms similar to this
issue.Yes, that can prevent the server from restarting at all. In this case,
to restart the server, we need to edit postgresql.auto.conf manually
and remove the problematic settings though the header of
postgresql.auto.conf warns "Do not edit this file manually!".So lets not try to forbid data_directory in postgresql.auto.conf and just
fix the case reported in this mail for postgresql.conf for which the
patch is attached upthread.
ISTM that data_directory is in different situation from the other. That is,
setting data_directory in postgresql.auto.conf is problematic whether its
setting value is valid or invalid. Imagine the case where data_directory
is set to '/data1' and '/data2' in config_directory/postgresql.conf and
/data1/postgresql.auto.conf, respectively. In this case, firstly the server
doesn't read /data2/postgresql.auto.conf when it starts up, even if your
patch has been applied. Secondly, the server doesn't read
/data1/postgresql.auto.conf when it receives SIGHUP, and this causes
the following error.
LOG: parameter "data_directory" cannot be changed without restarting the server
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, May 11, 2014 at 4:38 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
ISTM that data_directory is in different situation from the other. That is,
setting data_directory in postgresql.auto.conf is problematic whether its
setting value is valid or invalid. Imagine the case where data_directory
is set to '/data1' and '/data2' in config_directory/postgresql.conf and
/data1/postgresql.auto.conf, respectively. In this case, firstly the server
doesn't read /data2/postgresql.auto.conf when it starts up, even if your
patch has been applied.
I think after my patch, first server will read data1/postgresql.auto.conf
on start as that is what is set in postgresql.conf.
Secondly, the server doesn't read
/data1/postgresql.auto.conf when it receives SIGHUP, and this causes
the following error.LOG: parameter "data_directory" cannot be changed without restarting the server
This happens because data_directory is PGC_POSTMASTER parameter
and none of such parameters can be changed with SIGHUP.
In above scenario, I think you are expecting it should use
/data2/postgresql.auto.conf and that is what you have mentioned
up-thread. The way to handle it by server is just to forbid setting
this parameter
by Alter System or the user himself should not perform such an action.
Here if we want user to be careful of performing such an action, then may
be it's better to have such an indication in ALTER SYSTEM documentation.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila <amit.kapila16@gmail.com> writes:
In above scenario, I think you are expecting it should use
/data2/postgresql.auto.conf and that is what you have mentioned
up-thread. The way to handle it by server is just to forbid setting
this parameter
by Alter System or the user himself should not perform such an action.
Here if we want user to be careful of performing such an action, then may
be it's better to have such an indication in ALTER SYSTEM documentation.
I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf. The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.
We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory). It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila <amit.kapila16@gmail.com> writes:
This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start. The value of data_directory is only available after
processing of config files. To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set. Attached patch fixes the problem.
Aside from being about as ugly as it could possibly be, this is wrong,
because it only covers one of the possible sources of a data_directory
that's different from the config file location. In particular, it's
possible to set --config_file=something on the postmaster command line
and also set a data directory different than that via -D or a PGDATA
environment variable, rather than an entry in postgresql.conf (see the
initial logic in SelectConfigFiles).
While it's possible that you could deal with that with some even uglier
variant on this patch (perhaps involving making SelectConfigFiles export
its internal value of configdir), I think that having ProcessConfigFile
understand exactly what SelectConfigFiles is going to do to select the
final value of data_directory would be a clear modularity violation,
and very fragile as well.
I think what probably has to happen is that ProcessConfigFile shouldn't
be internally responsible for reading the auto file at all, but that we
do that via two separate calls to ProcessConfigFile, one for the main
file and then one for the auto file; and during initial startup,
SelectConfigFiles doesn't make the call for the auto file until after
it's established the final value of data_directory. (And by the by,
I think that DataDir not data_directory is what to be using to compute
the auto file's path.)
It's distressing that this wasn't noticed till now; it shows that nobody
tested ALTER SYSTEM with a config file outside the data directory, which
seems like a rather fundamental omission for any work with GUC files.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
I think what probably has to happen is that ProcessConfigFile shouldn't
be internally responsible for reading the auto file at all, but that we
do that via two separate calls to ProcessConfigFile, one for the main
file and then one for the auto file; and during initial startup,
SelectConfigFiles doesn't make the call for the auto file until after
it's established the final value of data_directory.
Since this bug would block testing of ALTER SYSTEM by a nontrivial
population of users, I felt it was important to get it fixed before beta,
so I went to try and fix it as above. It turns out that reading the two
config files separately doesn't work because ProcessConfigFile will think
all the settings got removed from the file. What we can do for the
moment, though, is to just run ProcessConfigFile twice during startup,
skipping the auto file the first time. It might be worth refactoring
ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
but it would be a lot of work and I think the gain would be marginal; in
any case there's not time to do that today. But I've got the main problem
fixed in time for beta.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, May 11, 2014 at 3:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
I think what probably has to happen is that ProcessConfigFile shouldn't
be internally responsible for reading the auto file at all, but that we
do that via two separate calls to ProcessConfigFile, one for the main
file and then one for the auto file; and during initial startup,
SelectConfigFiles doesn't make the call for the auto file until after
it's established the final value of data_directory.Since this bug would block testing of ALTER SYSTEM by a nontrivial
population of users, I felt it was important to get it fixed before beta,
so I went to try and fix it as above. It turns out that reading the two
config files separately doesn't work because ProcessConfigFile will think
all the settings got removed from the file. What we can do for the
moment, though, is to just run ProcessConfigFile twice during startup,
skipping the auto file the first time. It might be worth refactoring
ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
but it would be a lot of work and I think the gain would be marginal; in
any case there's not time to do that today. But I've got the main problem
fixed in time for beta.
Thanks for dealing with this. I was skeptical of the proposed patch,
but didn't have time to think hard enough about it to say something
intelligent. I'm glad you did.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, May 12, 2014 at 12:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Since this bug would block testing of ALTER SYSTEM by a nontrivial
population of users, I felt it was important to get it fixed before beta,
so I went to try and fix it as above. It turns out that reading the two
config files separately doesn't work because ProcessConfigFile will think
all the settings got removed from the file. What we can do for the
moment, though, is to just run ProcessConfigFile twice during startup,
skipping the auto file the first time. It might be worth refactoring
ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
but it would be a lot of work and I think the gain would be marginal; in
any case there's not time to do that today. But I've got the main problem
fixed in time for beta.
Thanks for fixing it.
I will provide a fix to forbid data_directory via Alter System as suggested
by you upthread.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.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 Sun, May 11, 2014 at 11:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf. The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory). It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.
I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
disallow parameters by Alter System and disallowed data_directory to
be set by Alter System.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
prohibit_data_dir_by_alter_system.patchapplication/octet-stream; name=prohibit_data_dir_by_alter_system.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1d094f0..fb962b7 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3098,7 +3098,7 @@ static struct config_string ConfigureNamesString[] =
{"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's data directory."),
NULL,
- GUC_SUPERUSER_ONLY
+ GUC_SUPERUSER_ONLY | GUC_DISALLOW_IN_AUTO_FILE
},
&data_directory,
NULL,
@@ -6736,11 +6736,12 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
errmsg("unrecognized configuration parameter \"%s\"", name)));
if ((record->context == PGC_INTERNAL) ||
- (record->flags & GUC_DISALLOW_IN_FILE))
- ereport(ERROR,
- (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
- errmsg("parameter \"%s\" cannot be changed",
- name)));
+ (record->flags & GUC_DISALLOW_IN_FILE) ||
+ (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
+ ereport(ERROR,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed",
+ name)));
if (!validate_conf_option(record, name, value, PGC_S_FILE,
ERROR, true, NULL,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c15a5bbb..f1a84e2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -195,6 +195,7 @@ typedef enum
#define GUC_UNIT_TIME 0x7000 /* mask for MS, S, MIN */
#define GUC_NOT_WHILE_SEC_REST 0x8000 /* can't set if security restricted */
+#define GUC_DISALLOW_IN_AUTO_FILE 0x00010000 /* can't set in postgresql.auto.conf */
/* GUC vars that are actually declared in guc.c, rather than elsewhere */
extern bool log_duration;
On Tue, May 27, 2014 at 10:35 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:
On Sun, May 11, 2014 at 11:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf. The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory). It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
disallow parameters by Alter System and disallowed data_directory to
be set by Alter System.
I have added this patch in next CF to avoid getting it lost.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 27, 2014 at 2:05 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Sun, May 11, 2014 at 11:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf. The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory). It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
disallow parameters by Alter System and disallowed data_directory to
be set by Alter System.
We should document what types of parameters are not allowed to be set by
ALTER SYSTEM SET?
data_directory was displayed when I typed "TAB" just after ALTER SYSTEM SET.
Probably tab-completion for ALTER SYSTEM SET needs to be changed.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jun 12, 2014 at 7:26 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, May 27, 2014 at 2:05 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:
On Sun, May 11, 2014 at 11:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf. The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory). It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
disallow parameters by Alter System and disallowed data_directory to
be set by Alter System.We should document what types of parameters are not allowed to be set by
ALTER SYSTEM SET?
Agreed, I had mentioned in Notes section of document. Apart from that
I had disallowed parameters that are excluded from postgresql.conf by
initdb (Developer options) and they are recommended in user manual
to be not used in production.
data_directory was displayed when I typed "TAB" just after ALTER SYSTEM
SET.
Probably tab-completion for ALTER SYSTEM SET needs to be changed.
This information is not stored in pg_settings. One way is to specify
manually all the parameters which are disallowed but it seems the query
will become clumsy, another could be to have another column in pg_settings.
Do you think of any other way?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
prohibit_data_dir_by_alter_system-v2.patchapplication/octet-stream; name=prohibit_data_dir_by_alter_system-v2.patchDownload
diff --git a/doc/src/sgml/ref/alter_system.sgml b/doc/src/sgml/ref/alter_system.sgml
index 081b372..af72d0d 100644
--- a/doc/src/sgml/ref/alter_system.sgml
+++ b/doc/src/sgml/ref/alter_system.sgml
@@ -77,6 +77,16 @@ ALTER SYSTEM SET <replaceable class="PARAMETER">configuration_parameter</replace
</refsect1>
<refsect1>
+ <title>Notes</title>
+
+ <para>
+ This command will not allow to set parameters that are disallowed or
+ excluded in postgresql.conf. It also disallows to set configuration
+ parameter <xref linkend="guc-data-directory">.
+ </para>
+ </refsect1>
+
+ <refsect1>
<title>Examples</title>
<para>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1d094f0..afcd2ce 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3095,10 +3095,14 @@ static struct config_string ConfigureNamesString[] =
},
{
+ /*
+ * Can't be set by Alter System as it can lead to recursive definition
+ * of data_directory.
+ */
{"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's data directory."),
NULL,
- GUC_SUPERUSER_ONLY
+ GUC_SUPERUSER_ONLY | GUC_DISALLOW_IN_AUTO_FILE
},
&data_directory,
NULL,
@@ -6735,12 +6739,18 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", name)));
+ /*
+ * Disallow parameter's that are excluded or disallowed in
+ * postgresql.conf.
+ */
if ((record->context == PGC_INTERNAL) ||
- (record->flags & GUC_DISALLOW_IN_FILE))
- ereport(ERROR,
- (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
- errmsg("parameter \"%s\" cannot be changed",
- name)));
+ (record->flags & GUC_DISALLOW_IN_FILE) ||
+ (record->flags & GUC_DISALLOW_IN_AUTO_FILE) ||
+ (record->flags & GUC_NOT_IN_SAMPLE))
+ ereport(ERROR,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed",
+ name)));
if (!validate_conf_option(record, name, value, PGC_S_FILE,
ERROR, true, NULL,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c15a5bbb..f1a84e2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -195,6 +195,7 @@ typedef enum
#define GUC_UNIT_TIME 0x7000 /* mask for MS, S, MIN */
#define GUC_NOT_WHILE_SEC_REST 0x8000 /* can't set if security restricted */
+#define GUC_DISALLOW_IN_AUTO_FILE 0x00010000 /* can't set in postgresql.auto.conf */
/* GUC vars that are actually declared in guc.c, rather than elsewhere */
extern bool log_duration;
Re: Amit Kapila 2014-06-13 <CAA4eK1KLn1SmgVtd=5emAbQxrrPVeEdTBUU94E-rEPMwxWVL+A@mail.gmail.com>
Agreed, I had mentioned in Notes section of document. Apart from that
I had disallowed parameters that are excluded from postgresql.conf by
initdb (Developer options) and they are recommended in user manual
to be not used in production.
Excluding developer options seems too excessive to me. ALTER SYSTEM
should be useful for developing too.
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Jun 15, 2014 at 6:29 PM, Christoph Berg <cb@df7cb.de> wrote:
Re: Amit Kapila 2014-06-13 <CAA4eK1KLn1SmgVtd=
5emAbQxrrPVeEdTBUU94E-rEPMwxWVL+A@mail.gmail.com>
Agreed, I had mentioned in Notes section of document. Apart from that
I had disallowed parameters that are excluded from postgresql.conf by
initdb (Developer options) and they are recommended in user manual
to be not used in production.Excluding developer options seems too excessive to me. ALTER SYSTEM
should be useful for developing too.
Developer options are mainly for debugging information or might help in one
of the situations, so I thought somebody might not want them to be part of
server configuration once they are set. We already disallow parameters like
config_file, transaction_isolation, etc. which are disallowed to be set in
postgresql.conf. Could you please explain me a bit in which
situations/scenarios, do you think that allowing developer options via Alter
System can be helpful?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: Amit Kapila 2014-06-16 <CAA4eK1++wcFswhzH=92qiQFB=C0_Uy=mReadvHZXdrF3JOWX6A@mail.gmail.com>
Developer options are mainly for debugging information or might help in one
of the situations, so I thought somebody might not want them to be part of
server configuration once they are set. We already disallow parameters like
config_file, transaction_isolation, etc. which are disallowed to be set in
postgresql.conf. Could you please explain me a bit in which
situations/scenarios, do you think that allowing developer options via Alter
System can be helpful?
Oh if these are disallowed in postgresql.conf, they should be
disallowed in auto.conf too. I meant to say that there shouldn't be
additional restrictions for auto.conf, except for cases like
data_directory which won't work at all (or are highly confusing).
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
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, Jun 16, 2014 at 12:14 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Sun, Jun 15, 2014 at 6:29 PM, Christoph Berg <cb@df7cb.de> wrote:
Re: Amit Kapila 2014-06-13
<CAA4eK1KLn1SmgVtd=5emAbQxrrPVeEdTBUU94E-rEPMwxWVL+A@mail.gmail.com>Agreed, I had mentioned in Notes section of document. Apart from that
I had disallowed parameters that are excluded from postgresql.conf by
initdb (Developer options) and they are recommended in user manual
to be not used in production.Excluding developer options seems too excessive to me. ALTER SYSTEM
should be useful for developing too.Developer options are mainly for debugging information or might help in one
of the situations, so I thought somebody might not want them to be part of
server configuration once they are set. We already disallow parameters like
config_file, transaction_isolation, etc. which are disallowed to be set in
postgresql.conf. Could you please explain me a bit in which
situations/scenarios, do you think that allowing developer options via Alter
System can be helpful?
I think that's helpful. I've heard that some users enable the developer option
"trace_sort" in postgresql.conf in order to collect the information
about sorting.
They might want to set trace_sort via ALTER SYSTEM, for example.
This information is not stored in pg_settings. One way is to specify
manually all the parameters which are disallowed but it seems the query
will become clumsy, another could be to have another column in pg_settings.
Do you think of any other way?
I like the latter idea, i.e., exposing the flags of GUC parameters in
pg_settings,
but it seems overkill just for tab-completion purpose. I'd withdraw my comment.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi.
Just a few minor comments about your patch:
At 2014-06-13 11:46:21 +0530, amit.kapila16@gmail.com wrote:
+ <title>Notes</title> + + <para> + This command will not allow to set parameters that are disallowed or + excluded in postgresql.conf. It also disallows to set configuration + parameter <xref linkend="guc-data-directory">. + </para> + </refsect1>
I suggest the following wording:
This command may not be used to set
<xref linkend="guc-data-directory">
or any parameters that are not allowed in postgresql.conf.
+ /* + * Disallow parameter's that are excluded or disallowed in + * postgresql.conf. + */
"parameters", no apostrophe.
if ((record->context == PGC_INTERNAL) || - (record->flags & GUC_DISALLOW_IN_FILE)) - ereport(ERROR, - (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), - errmsg("parameter \"%s\" cannot be changed", - name))); + (record->flags & GUC_DISALLOW_IN_FILE) || + (record->flags & GUC_DISALLOW_IN_AUTO_FILE) || + (record->flags & GUC_NOT_IN_SAMPLE)) + ereport(ERROR, + (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), + errmsg("parameter \"%s\" cannot be changed", + name)));
I looked at the settings that are marked GUC_NOT_IN_SAMPLE but neither
PGC_INTERNAL nor GUC_DISALLOW_IN_*FILE. I don't feel strongly about it,
but I don't see any particularly good reason to exclude them here.
(I also agree with Fujii-san that it isn't worth making extensive
changes to avoid data_directory being offered via tab-completion.)
-- Abhijit
--
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, Jun 16, 2014 at 7:06 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Developer options are mainly for debugging information or might help in
one
of the situations, so I thought somebody might not want them to be part
of
server configuration once they are set. We already disallow parameters
like
config_file, transaction_isolation, etc. which are disallowed to be set
in
postgresql.conf. Could you please explain me a bit in which
situations/scenarios, do you think that allowing developer options via
Alter
System can be helpful?
I think that's helpful. I've heard that some users enable the developer
option
"trace_sort" in postgresql.conf in order to collect the information
about sorting.
They might want to set trace_sort via ALTER SYSTEM, for example.
Okay, if you have usecase where people use such parameters in
postegresql.conf, then it makes sense. I have removed that restriction
from patch.
I suggest the following wording:
This command may not be used to set
<xref linkend="guc-data-directory">
or any parameters that are not allowed in postgresql.conf.
I think we should not use *may* in this line, because in no
circumstance we will allow this command for the parameters
mentioned. I have changed it as per your suggestion.
+ /* + * Disallow parameter's that are excluded or disallowed in + * postgresql.conf. + */"parameters", no apostrophe.
okay.
I have changed above comment as earlier comment has
information about developer option which we don't need now.
if ((record->context == PGC_INTERNAL) ||
- (record->flags & GUC_DISALLOW_IN_FILE))
- ereport(ERROR,
-
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
- errmsg("parameter \"%s\" cannot be
changed",
- name))); + (record->flags & GUC_DISALLOW_IN_FILE) || + (record->flags & GUC_DISALLOW_IN_AUTO_FILE) || + (record->flags & GUC_NOT_IN_SAMPLE)) + ereport(ERROR, +
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be
changed",
+ name)));
I looked at the settings that are marked GUC_NOT_IN_SAMPLE but neither
PGC_INTERNAL nor GUC_DISALLOW_IN_*FILE. I don't feel strongly about it,
but I don't see any particularly good reason to exclude them here.
By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
something else, if earlier then I have removed it as per comment from
Fujji-san.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
prohibit_data_dir_by_alter_system-v3.patchapplication/octet-stream; name=prohibit_data_dir_by_alter_system-v3.patchDownload
diff --git a/doc/src/sgml/ref/alter_system.sgml b/doc/src/sgml/ref/alter_system.sgml
index 081b372..4918596 100644
--- a/doc/src/sgml/ref/alter_system.sgml
+++ b/doc/src/sgml/ref/alter_system.sgml
@@ -77,6 +77,15 @@ ALTER SYSTEM SET <replaceable class="PARAMETER">configuration_parameter</replace
</refsect1>
<refsect1>
+ <title>Notes</title>
+
+ <para>
+ This command can't be used to set <xref linkend="guc-data-directory">
+ or any parameters that are not allowed in postgresql.conf.
+ </para>
+ </refsect1>
+
+ <refsect1>
<title>Examples</title>
<para>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1d094f0..a29231d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3095,10 +3095,14 @@ static struct config_string ConfigureNamesString[] =
},
{
+ /*
+ * Can't be set by Alter System as it can lead to recursive definition
+ * of data_directory.
+ */
{"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
gettext_noop("Sets the server's data directory."),
NULL,
- GUC_SUPERUSER_ONLY
+ GUC_SUPERUSER_ONLY | GUC_DISALLOW_IN_AUTO_FILE
},
&data_directory,
NULL,
@@ -6735,12 +6739,17 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", name)));
+ /*
+ * Don't allow internal parameters that can't be set by user or
+ * are not allowed in postgresql.conf/postgresql.auto.conf.
+ */
if ((record->context == PGC_INTERNAL) ||
- (record->flags & GUC_DISALLOW_IN_FILE))
- ereport(ERROR,
- (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
- errmsg("parameter \"%s\" cannot be changed",
- name)));
+ (record->flags & GUC_DISALLOW_IN_FILE) ||
+ (record->flags & GUC_DISALLOW_IN_AUTO_FILE))
+ ereport(ERROR,
+ (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+ errmsg("parameter \"%s\" cannot be changed",
+ name)));
if (!validate_conf_option(record, name, value, PGC_S_FILE,
ERROR, true, NULL,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c15a5bbb..f1a84e2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -195,6 +195,7 @@ typedef enum
#define GUC_UNIT_TIME 0x7000 /* mask for MS, S, MIN */
#define GUC_NOT_WHILE_SEC_REST 0x8000 /* can't set if security restricted */
+#define GUC_DISALLOW_IN_AUTO_FILE 0x00010000 /* can't set in postgresql.auto.conf */
/* GUC vars that are actually declared in guc.c, rather than elsewhere */
extern bool log_duration;
At 2014-06-17 09:49:35 +0530, amit.kapila16@gmail.com wrote:
By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
something else, if earlier then I have removed it as per comment from
Fujji-san.
Yes, what you've done in v3 of the patch is what I meant. I've marked
this as ready for committer now.
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jun 17, 2014 at 2:08 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2014-06-17 09:49:35 +0530, amit.kapila16@gmail.com wrote:
By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
something else, if earlier then I have removed it as per comment from
Fujji-san.Yes, what you've done in v3 of the patch is what I meant. I've marked
this as ready for committer now.
Barring objections, I will commit this patch.
Also I'm thinking to backpatch this to 9.4 because this is a bugfix rather
than new feature.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
At 2014-06-18 12:55:36 +0900, masao.fujii@gmail.com wrote:
Also I'm thinking to backpatch this to 9.4 because this is a bugfix
rather than new feature.
That sounds reasonable, thanks.
-- Abhijit
--
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, Jun 18, 2014 at 1:07 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2014-06-18 12:55:36 +0900, masao.fujii@gmail.com wrote:
Also I'm thinking to backpatch this to 9.4 because this is a bugfix
rather than new feature.That sounds reasonable, thanks.
Committed!
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jun 19, 2014 at 5:21 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Jun 18, 2014 at 1:07 PM, Abhijit Menon-Sen <ams@2ndquadrant.com>
wrote:
At 2014-06-18 12:55:36 +0900, masao.fujii@gmail.com wrote:
Also I'm thinking to backpatch this to 9.4 because this is a bugfix
rather than new feature.That sounds reasonable, thanks.
Committed!
Thank you.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com