Re: [RFC] Extend namespace of valid guc names

Hi,

Currently guc-file.c allows the following as guc names:

ID {LETTER}{LETTER_OR_DIGIT}*
QUALIFIED_ID {ID}"."{ID}

That is either one token starting with a letter followed by numbers or
letters or exactly two of those separated by a dot.
Those restrictions are existing for neither SET/set_config() via SQL nor
for postgres -c styles of setting options.

I propose loosening those restrictions to
a) allow repeatedly qualified names like a.b.c
b) allow variables to start with a digit from the second level onwards.

Additionally, should we perhaps enforce the same rules for -c and
set_config()/SET?

Trivial patch that only extends the space of valid names for
postgresql.conf attached.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hello

Why? There are no multilevels structures in pg. Variables should be joined
with schemas or extensions. Other levels are messy.

On 2013-02-25 22:27:21 +0100, Pavel Stehule wrote:
> Why?

The concrete usecase is having some form of nesting available for a
shared_library.

shared_preload_libraries = 'bdr'
bdr.connections = 'a, b'
bdr.a.dsn = '...'
bdr.a.xxx = '...'
bdr.b.dsn = '...'

> There are no multilevels structures in pg. Variables should be joined
> with schemas or extensions. Other levels are messy.

So? What does the one existing level SQL have to do with
postgresql.conf? And why is it messy?

Not that as I said before
SET foo.bar.blub = '1'; currently is already allowed.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 02/25/2013 04:34 PM, Andres Freund wrote:

> Not that as I said before
> SET foo.bar.blub = '1'; currently is already allowed.

I gather you mean "Note". I agree that it seems very odd to allow this
in one context and forbid it in another.

cheers

andrew

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> I propose loosening those restrictions to
> a) allow repeatedly qualified names like a.b.c

If SET allows it, I guess we can allow it here. But is a grammar change
really all that is needed to make it work from the file?

> b) allow variables to start with a digit from the second level onwards.

That seems like a seriously bad idea. I note that SET does *not* allow
this; furthermore it seems like a considerable weakening of our ability
to detect silly typos in config files. Nor did you offer a use-case
to justify it.

regards, tom lane

On 2013-02-25 21:13:25 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > I propose loosening those restrictions to
> > a) allow repeatedly qualified names like a.b.c
>
> If SET allows it, I guess we can allow it here. But is a grammar change
> really all that is needed to make it work from the file?

Seems so. There's no additional validation that I could find
anywhere. And a simple test confirmed it works.

postgres=# SHOW foo.bar.blub;
foo.bar.blub
--------------
1
(1 row)

Just for reference, thats the grammar for SET/SHOW:

var_name: ColId { $$ = $1; }
| var_name '.' ColId

> > b) allow variables to start with a digit from the second level onwards.
>
> That seems like a seriously bad idea. I note that SET does *not* allow
> this; furthermore it seems like a considerable weakening of our ability
> to detect silly typos in config files. Nor did you offer a use-case
> to justify it.

The use-case I had in mind was

bdr.1.dsn = ...
bdr.2.dsn = ...
bdr.3.dsn = ...
bdr.4.dsn = ...

which is what I had used via -c. But I guess it can easy enough be
replaced by node_$i or something.

Any arguments whether we should try to validate -c SET/SHOW,
set_config() and -c the same?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2013-02-26 13:08:54 +0100, Andres Freund wrote:
> Just for reference, thats the grammar for SET/SHOW:
>
> var_name: ColId { $$ = $1; }
> | var_name '.' ColId
>
> Any arguments whether we should try to validate postgres -c,
> set_config and SET/SHOW the same?

Any input here?

Currently set_config() and postgres -c basically don't have any
restrictions on the passed guc name whereas SHOW, SET and
postgresql.conf do.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2013-02-25 21:13:25 -0500, Tom Lane wrote:
> > b) allow variables to start with a digit from the second level onwards.
>
> That seems like a seriously bad idea. I note that SET does *not* allow
> this;

Hm. One thing about this is that we currently allow something silly as:
SET "1"."1bar""blub" = 3;

So I'd like to either restrict SET here or allow the same for guc-file.l
parsed GUCs. Any opinions?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-02-25 21:13:25 -0500, Tom Lane wrote:
>>> b) allow variables to start with a digit from the second level onwards.

>> That seems like a seriously bad idea. I note that SET does *not* allow
>> this;

> Hm. One thing about this is that we currently allow something silly as:
> SET "1"."1bar""blub" = 3;

> So I'd like to either restrict SET here or allow the same for guc-file.l
> parsed GUCs. Any opinions?

Well, if you feel an absolute compulsion to make them consistent, I'd
go with making SET disallow creation of variables with names the file
parser wouldn't recognize. But why is it such a bad thing if SET can
do that? The whole reason we allow SET to create new variables at all
is that the universe of things you can have as session-local values is
larger than the set of parameters that are allowed in postgresql.conf.
So I'm missing why we need such a restriction.

regards, tom lane

On 2013-09-06 10:13:23 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> >> That seems like a seriously bad idea. I note that SET does *not* allow
> >> this;
>
> > Hm. One thing about this is that we currently allow something silly as:
> > SET "1"."1bar""blub" = 3;
>
> > So I'd like to either restrict SET here or allow the same for guc-file.l
> > parsed GUCs. Any opinions?
>
> Well, if you feel an absolute compulsion to make them consistent, I'd
> go with making SET disallow creation of variables with names the file
> parser wouldn't recognize. But why is it such a bad thing if SET can
> do that? The whole reason we allow SET to create new variables at all
> is that the universe of things you can have as session-local values is
> larger than the set of parameters that are allowed in postgresql.conf.
> So I'm missing why we need such a restriction.

Well, it's confusing for users, i.e. me. I've several times now
prototyped stuff that was supposed to be configurable in postgresql.conf
by either passing the options to postgres -c or by doing user level
SETs. Only to then later discover that what I've prototyped doesn't work
because the restrictions in postgresql.conf are way stricter.

Also, ALTER SYSTEM SET is going to need a similar restriction as well,
otherwise the server won't restart although the GUCs pass validation...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-09-06 10:13:23 -0400, Tom Lane wrote:
>> Well, if you feel an absolute compulsion to make them consistent, I'd
>> go with making SET disallow creation of variables with names the file
>> parser wouldn't recognize. But why is it such a bad thing if SET can
>> do that?

> Also, ALTER SYSTEM SET is going to need a similar restriction as well,
> otherwise the server won't restart although the GUCs pass validation...

Well, sure, but I would think that ALTER SYSTEM SET should be constrained
to only set known GUCs, not invent new ones on the fly.

regards, tom lane

On 09/06/2013 08:48 PM, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> On 2013-09-06 10:13:23 -0400, Tom Lane wrote:
>>> Well, if you feel an absolute compulsion to make them consistent, I'd
>>> go with making SET disallow creation of variables with names the file
>>> parser wouldn't recognize. But why is it such a bad thing if SET can
>>> do that?
>> Also, ALTER SYSTEM SET is going to need a similar restriction as well,
>> otherwise the server won't restart although the GUCs pass validation...
> Well, sure, but I would think that ALTER SYSTEM SET should be constrained
> to only set known GUCs, not invent new ones on the fly.
What's the reasoning behind this ?

I was assuming that ALTER SYSTEM SET would allow all GUCs which
do not require restart which includes all "newly invented" ones.

Cheers

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> On 09/06/2013 08:48 PM, Tom Lane wrote:
>> Well, sure, but I would think that ALTER SYSTEM SET should be constrained
>> to only set known GUCs, not invent new ones on the fly.

> What's the reasoning behind this ?

If you don't know what a GUC is, you don't know what are valid values for
it, and thus you might write an illegal value into auto.conf (or whatever
we're calling it this week). That could have consequences as bad as
failure to restart, should the DBA decide to preload the module defining
that GUC, which would then complain about the bad value during postmaster
start.

> I was assuming that ALTER SYSTEM SET would allow all GUCs which
> do not require restart which includes all "newly invented" ones.

I do not believe that the former need imply the latter, nor do I see a
strong use-case for allowing ALTER SYSTEM SET on session-local GUCs,
which is what any truly invented-on-the-fly GUCs would be. The whole
business with session-local GUCs is pretty much a kluge anyway, which
we might want to retire or redefine someday; so I'd much prefer that
ALTER SYSTEM SET stayed out of it.

regards, tom lane

On 2013-09-06 14:48:33 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2013-09-06 10:13:23 -0400, Tom Lane wrote:
> >> Well, if you feel an absolute compulsion to make them consistent, I'd
> >> go with making SET disallow creation of variables with names the file
> >> parser wouldn't recognize. But why is it such a bad thing if SET can
> >> do that?
>
> > Also, ALTER SYSTEM SET is going to need a similar restriction as well,
> > otherwise the server won't restart although the GUCs pass validation...
>
> Well, sure, but I would think that ALTER SYSTEM SET should be constrained
> to only set known GUCs, not invent new ones on the fly.

Hm. That sounds inconvenient to me. Consider something like configuring
the system to use auto_explain henceforth.
ALTER SYSTEM SET shared_preload_libraries = 'auto_explain';
ALTER SYSTEM SET auto_explain.log_min_duration = 100;

It seems weird to forbid doing that and requiring a manual LOAD when we
don't do so for normal SETs. I can live with the restriction if we
decide it's a good idea, I just wouldn't appreciate it.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Sep 6, 2013 at 6:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-09-06 14:48:33 -0400, Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> > On 2013-09-06 10:13:23 -0400, Tom Lane wrote:
>> >> Well, if you feel an absolute compulsion to make them consistent, I'd
>> >> go with making SET disallow creation of variables with names the file
>> >> parser wouldn't recognize. But why is it such a bad thing if SET can
>> >> do that?
>>
>> > Also, ALTER SYSTEM SET is going to need a similar restriction as well,
>> > otherwise the server won't restart although the GUCs pass validation...
>>
>> Well, sure, but I would think that ALTER SYSTEM SET should be constrained
>> to only set known GUCs, not invent new ones on the fly.
>
> Hm. That sounds inconvenient to me. Consider something like configuring
> the system to use auto_explain henceforth.
> ALTER SYSTEM SET shared_preload_libraries = 'auto_explain';
> ALTER SYSTEM SET auto_explain.log_min_duration = 100;
>
> It seems weird to forbid doing that and requiring a manual LOAD when we
> don't do so for normal SETs. I can live with the restriction if we
> decide it's a good idea, I just wouldn't appreciate it.

I'm with Tom on this one: I think this will save more pain than it causes.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Fri, Sep 6, 2013 at 6:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> On 2013-09-06 14:48:33 -0400, Tom Lane wrote:
>>> Well, sure, but I would think that ALTER SYSTEM SET should be constrained
>>> to only set known GUCs, not invent new ones on the fly.

>> Hm. That sounds inconvenient to me. Consider something like configuring
>> the system to use auto_explain henceforth.
>> ALTER SYSTEM SET shared_preload_libraries = 'auto_explain';
>> ALTER SYSTEM SET auto_explain.log_min_duration = 100;

> I'm with Tom on this one: I think this will save more pain than it causes.

So far as that example goes, I'm not suggesting that "ALTER SYSTEM SET
auto_explain.log_min_duration" should be forbidden altogether. I *am*
saying that it should only be allowed when auto_explain is loaded in the
current session, so that we can find out whether the proposed value is
allowed by the module that defines the GUC.

Of course, this is not completely bulletproof, since it will fail if the
defining module changes its mind from time to time about what are valid
values of the GUC :-(. But promising to restart in the face of that kind
of inconsistency is hopeless. On the other hand, not checking at all is
just asking for failures.

regards, tom lane

On 2013-02-25 22:15:33 +0100, Andres Freund wrote:
> Currently guc-file.c allows the following as guc names:
>
> ID {LETTER}{LETTER_OR_DIGIT}*
> QUALIFIED_ID {ID}"."{ID}
>
> That is either one token starting with a letter followed by numbers or
> letters or exactly two of those separated by a dot.
> Those restrictions are existing for neither SET/set_config() via SQL nor
> for postgres -c styles of setting options.
>
> I propose loosening those restrictions to
> a) allow repeatedly qualified names like a.b.c
> b) allow variables to start with a digit from the second level onwards.

The attached patch does a) but not b) as Tom argued it's not allowed in
a plain fashion in SQL either. There you need to quote the variable name.

> Additionally, should we perhaps enforce the same rules for -c and
> set_config()/SET?

The discussion seems to have concluded that this isn't neccessary, so I
haven't included this.

The patch still is pretty trivial - I've looked around and I really
didn't see anything else that requires changes. I haven't even found
documentation to adapt.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> a) allow repeatedly qualified names like a.b.c
>
> The attached patch does a)

What is the use case for this change?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
> On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> a) allow repeatedly qualified names like a.b.c
> >
> > The attached patch does a)
>
> What is the use case for this change?

Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
or the commit message of the patch.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
>> On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> >> a) allow repeatedly qualified names like a.b.c
>> >
>> > The attached patch does a)
>>
>> What is the use case for this change?
>
> Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
> or the commit message of the patch.

That's more or less what I figured. One thing I'm uncomfortable with
is that, while this is useful for extensions, what do we do when we
have a similar requirement for core? The proposed GUC name is of the
format extension.user_defined_object_name.property_name; but omitting
the extension name would lead to
user_defined_object_name.property_name, which doesn't feel good as a
method for naming core GUCs. The user defined object name might just
so happen to be an extension name.

More generally, it seems like this is trying to take structured data
and fit into the GUC mechanism, and I'm not sure that's going to be a
real good fit. I mean, pg_hba.conf could be handled this way. You
could have hba.1.type = local, hba.1.database = all, hba.1.user = all,
hba.2.type = host, etc. But I doubt that any of us would consider
that an improvement over the actual approach of having a separate file
with that information. If it's not a good fit for pg_hba.conf, why is
it a good fit for anything else we want to do?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2013-09-17 16:24:34 -0400, Robert Haas wrote:
> On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
> >> What is the use case for this change?
> >
> > Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
> > or the commit message of the patch.
>
> That's more or less what I figured. One thing I'm uncomfortable with
> is that, while this is useful for extensions, what do we do when we
> have a similar requirement for core? The proposed GUC name is of the
> format extension.user_defined_object_name.property_name; but omitting
> the extension name would lead to
> user_defined_object_name.property_name, which doesn't feel good as a
> method for naming core GUCs. The user defined object name might just
> so happen to be an extension name.

I think that ship has long since sailed. postgresql.conf has allowed
foo.bar style GUCs via custom_variable_classes for a long time, and
these days we don't even require that but allow them generally. Also,
SET, postgres -c, and SELECT set_config() already don't have the
restriction to one dot in the variable name.

I think if we're going to use something like that for postgres, we'll
have to live with the chance of breaking applications (although I don't
think it's that big for most of the variables where I can forsee the use).

> If it's not a good fit for pg_hba.conf, why is it a good fit for
> anything else we want to do?

Well, for now it's primarily there for extensions, not for core
code. Although somebody has already asked when I'd submit the patch
because they could use it for their patch...
I don't really find the pg_hba.conf comparison terribly convincing. As
an extension, how would you prefer to formulate the configuration
in the example?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Wed, Sep 18, 2013 at 4:26 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> Hi,
>
> On 2013-09-17 16:24:34 -0400, Robert Haas wrote:
>> On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
>> >> What is the use case for this change?
>> >
>> > Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
>> > or the commit message of the patch.
>>
>> That's more or less what I figured. One thing I'm uncomfortable with
>> is that, while this is useful for extensions, what do we do when we
>> have a similar requirement for core? The proposed GUC name is of the
>> format extension.user_defined_object_name.property_name; but omitting
>> the extension name would lead to
>> user_defined_object_name.property_name, which doesn't feel good as a
>> method for naming core GUCs. The user defined object name might just
>> so happen to be an extension name.
>
> I think that ship has long since sailed. postgresql.conf has allowed
> foo.bar style GUCs via custom_variable_classes for a long time, and
> these days we don't even require that but allow them generally. Also,
> SET, postgres -c, and SELECT set_config() already don't have the
> restriction to one dot in the variable name.

It's even explained in document that a two-part name is allowed for
Customized Options at link:
http://www.postgresql.org/docs/devel/static/runtime-config-custom.html

> I think if we're going to use something like that for postgres, we'll
> have to live with the chance of breaking applications (although I don't
> think it's that big for most of the variables where I can forsee the use).
>
>> If it's not a good fit for pg_hba.conf, why is it a good fit for
>> anything else we want to do?
>
> Well, for now it's primarily there for extensions, not for core
> code. Although somebody has already asked when I'd submit the patch
> because they could use it for their patch...

I had reviewed and tested the patch.

The purpose of patch is to allow more than two-part parameter name
through postgresql.conf.

I think it's useful to allow more than two-parameter names mainly because
a) in database terminology, people are quite use to think for more
than two-part names,
for ex. any object <database_name>.<schema_name>.<object_name>
here object_name can be table_name, index_name, etc.
b) it already works in SET/set_config().

The code is fine and I had done tests for various parameters (more
than two-part) in postgresql.conf all of which works fine (Select
pg_reload_conf() and show of parameter name).
For example:

foo2.bar2.bar1 = 3
foo2.bar2.bar1 = 4
foo2.bar2.bar1.bar1 = 4
a.b._c = 4
d.e.f =on
d.e.f.g =true
d.e.f.g.h = 'abcdef'
a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z
= 4

Apart from this, quite a few negative tests (setting invalid names)
also works fine (select pg_reload_conf() shows error on server).

We can modify the documentation for Customized Options to mention
about support for more than two-part name.

On a side note, although it doesn't have any relation with your patch,
I found some minor problems in setting of configuration during test of
this patch, so I am mentioning it here. I will look into these in
detail later:

Test-1
postgres=# select set_config('a.b.1.c','c',false);
set_config
------------
c
(1 row)

postgres=# show a.b.1.c;
ERROR: syntax error at or near ".1"
LINE 1: show a.b.1.c;

allows to set variable name which has integer, but could not see it
via show as other variables can be seen.

Test-2
test to see if longer length string are compatible in all interfaces.
set fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.bar=1;

Behavior of set command
-------------------------
postgres=# set foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.bar=1;
NOTICE: identifier "foooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" will be tru
ncated to "foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
SET
postgres=# show foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.
bar;
foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
-----------------------------------------------------------------
1
(1 row)

Behavior when the same is set though postgresql.conf
----------------------------------------------------
postgres=# show fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo.bar;
NOTICE: identifier "foooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" will be tru
ncated to "foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
ERROR: unrecognized configuration parameter "fooooooooooooooooooooooooooooooooo
ooooooooooooooooooooooooooooo.bar"
postgres=#

Although variable is loaded but you cannot see it.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 2013-09-18 11:55:24 +0530, Amit Kapila wrote:

> > I think that ship has long since sailed. postgresql.conf has allowed
> > foo.bar style GUCs via custom_variable_classes for a long time, and
> > these days we don't even require that but allow them generally. Also,
> > SET, postgres -c, and SELECT set_config() already don't have the
> > restriction to one dot in the variable name.
>
> It's even explained in document that a two-part name is allowed for
> Customized Options at link:
> http://www.postgresql.org/docs/devel/static/runtime-config-custom.html

Oh I somehow missed that. I'll need to patch that as well.

> Apart from this, quite a few negative tests (setting invalid names)
> also works fine (select pg_reload_conf() shows error on server).

> On a side note, although it doesn't have any relation with your patch,
> I found some minor problems in setting of configuration during test of
> this patch, so I am mentioning it here. I will look into these in
> detail later:

Most of those have been discussed in another subthread. While I still am
not sure that I agree, the concensus was that we shouldn't do anything
about that for now.

>
> Test-1
> postgres=# select set_config('a.b.1.c','c',false);
> set_config
> ------------
> c
> (1 row)
>
> postgres=# show a.b.1.c;
> ERROR: syntax error at or near ".1"
> LINE 1: show a.b.1.c;

You can show it with a.b."1".c, but those can't be set via guc-file.l.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2013-09-18 11:02:50 +0200, Andres Freund wrote:
> On 2013-09-18 11:55:24 +0530, Amit Kapila wrote:
>
> > > I think that ship has long since sailed. postgresql.conf has allowed
> > > foo.bar style GUCs via custom_variable_classes for a long time, and
> > > these days we don't even require that but allow them generally. Also,
> > > SET, postgres -c, and SELECT set_config() already don't have the
> > > restriction to one dot in the variable name.
> >
> > It's even explained in document that a two-part name is allowed for
> > Customized Options at link:
> > http://www.postgresql.org/docs/devel/static/runtime-config-custom.html
>
> Oh I somehow missed that. I'll need to patch that as well.

Updated patch attached.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> That's more or less what I figured. One thing I'm uncomfortable with
>> is that, while this is useful for extensions, what do we do when we
>> have a similar requirement for core? The proposed GUC name is of the
>> format extension.user_defined_object_name.property_name; but omitting
>> the extension name would lead to
>> user_defined_object_name.property_name, which doesn't feel good as a
>> method for naming core GUCs. The user defined object name might just
>> so happen to be an extension name.
>
> I think that ship has long since sailed. postgresql.conf has allowed
> foo.bar style GUCs via custom_variable_classes for a long time, and
> these days we don't even require that but allow them generally. Also,
> SET, postgres -c, and SELECT set_config() already don't have the
> restriction to one dot in the variable name.

Well, we should make it consistent, but I'm not sure that settles the
question of which direction to go.

>> If it's not a good fit for pg_hba.conf, why is it a good fit for
>> anything else we want to do?
>
> Well, for now it's primarily there for extensions, not for core
> code. Although somebody has already asked when I'd submit the patch
> because they could use it for their patch...
> I don't really find the pg_hba.conf comparison terribly convincing. As
> an extension, how would you prefer to formulate the configuration
> in the example?

Well, to me it looks like what you've got there is a table. And I
don't have a clear feeling of how that ought to be represented in
configuration-land, but trying to jam it into the GUC machinery seems
awkward at best. I think we need a way of handling tabular
configuration data, but I'm not convinced this is it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2013-09-19 14:57:53 -0400, Robert Haas wrote:
> On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > I think that ship has long since sailed. postgresql.conf has allowed
> > foo.bar style GUCs via custom_variable_classes for a long time, and
> > these days we don't even require that but allow them generally. Also,
> > SET, postgres -c, and SELECT set_config() already don't have the
> > restriction to one dot in the variable name.
>
> Well, we should make it consistent, but I'm not sure that settles the
> question of which direction to go.

I suggested making it consistent in either form elsewhere in this
thread, Tom wasn't convinced. I think because of backward compatibility
concerns we hardly can restrict the valid namespace in all forms to the
most restrictive form (i.e. guc-file.l). Quite some people use GUCs as
variables.
Maybe we can forbid the more absurd variants (SET "..."."..." = 3;
SELECT set_config('...', '1', true)), but restricting it entirely seems
to cause pain for no reason.

> >> If it's not a good fit for pg_hba.conf, why is it a good fit for
> >> anything else we want to do?
> >
> > Well, for now it's primarily there for extensions, not for core
> > code. Although somebody has already asked when I'd submit the patch
> > because they could use it for their patch...
> > I don't really find the pg_hba.conf comparison terribly convincing. As
> > an extension, how would you prefer to formulate the configuration
> > in the example?
>
> Well, to me it looks like what you've got there is a table. And I
> don't have a clear feeling of how that ought to be represented in
> configuration-land, but trying to jam it into the GUC machinery seems
> awkward at best. I think we need a way of handling tabular
> configuration data, but I'm not convinced this is it.

Well, it has two major advantages from my pov: a) we already have it
from SQL and people already use it that way b) it's dirt simple and it
doesn't allow anything not allowed before via other means.

Maybe you can invent something nicer to look at that's also parsed
before the server starts, but I find it hard to see the need
TBH. Especially as you will want most of the infrastructure GUCs
provide...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Thu, Sep 19, 2013 at 2:48 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-09-18 11:02:50 +0200, Andres Freund wrote:
>> On 2013-09-18 11:55:24 +0530, Amit Kapila wrote:
>>
>> > > I think that ship has long since sailed. postgresql.conf has allowed
>> > > foo.bar style GUCs via custom_variable_classes for a long time, and
>> > > these days we don't even require that but allow them generally. Also,
>> > > SET, postgres -c, and SELECT set_config() already don't have the
>> > > restriction to one dot in the variable name.
>> >
>> > It's even explained in document that a two-part name is allowed for
>> > Customized Options at link:
>> > http://www.postgresql.org/docs/devel/static/runtime-config-custom.html
>>
>> Oh I somehow missed that. I'll need to patch that as well.
>
> Updated patch attached.

old part of line
- PostgreSQL will accept a setting for any two-part parameter name.

new part of line
+ PostgreSQL will accept a setting for any parameter name containing
at least one dot.

If I read new part of line just in context of custom options, it makes
sense, but when I compare with old line I think below lines or variant
of them might make more sense as this line is not restricted to just
custom options:

a. PostgreSQL will accept a setting for any parameter name containing
two or more part parameter names.
b. PostgreSQL will accept a setting for any parameter name containing
one or more dots in parts of parameter names.

It's just how user interpret any line, so may be your line is more
meaningful to some users. If you don't think there is any need to
change then keep as it is and let committer decide about it. I don't
have any big problem with the current wording.

I think Robert is still not fully convinced about this patch, but from
my side review of patch with the current scope is complete, so I will
mark it as Ready For Committer if nobody has objections for the same.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Thu, Sep 19, 2013 at 6:19 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-09-19 14:57:53 -0400, Robert Haas wrote:
>> On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > I think that ship has long since sailed. postgresql.conf has allowed
>> > foo.bar style GUCs via custom_variable_classes for a long time, and
>> > these days we don't even require that but allow them generally. Also,
>> > SET, postgres -c, and SELECT set_config() already don't have the
>> > restriction to one dot in the variable name.
>>
>> Well, we should make it consistent, but I'm not sure that settles the
>> question of which direction to go.
>
> I suggested making it consistent in either form elsewhere in this
> thread, Tom wasn't convinced. I think because of backward compatibility
> concerns we hardly can restrict the valid namespace in all forms to the
> most restrictive form (i.e. guc-file.l). Quite some people use GUCs as
> variables.
> Maybe we can forbid the more absurd variants (SET "..."."..." = 3;
> SELECT set_config('...', '1', true)), but restricting it entirely seems
> to cause pain for no reason.

Yeah, I don't think I understand Tom's reasoning. I mean, why
shouldn't there be ONE rule for what can be a GUC?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Sep 20, 2013 at 9:48 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Thu, Sep 19, 2013 at 2:48 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> On 2013-09-18 11:02:50 +0200, Andres Freund wrote:
>>> On 2013-09-18 11:55:24 +0530, Amit Kapila wrote:
>>>
>>> > > I think that ship has long since sailed. postgresql.conf has allowed
>>> > > foo.bar style GUCs via custom_variable_classes for a long time, and
>>> > > these days we don't even require that but allow them generally. Also,
>>> > > SET, postgres -c, and SELECT set_config() already don't have the
>>> > > restriction to one dot in the variable name.
>>> >
>>> > It's even explained in document that a two-part name is allowed for
>>> > Customized Options at link:
>>> > http://www.postgresql.org/docs/devel/static/runtime-config-custom.html
>>>
>>> Oh I somehow missed that. I'll need to patch that as well.
>>
>> Updated patch attached.
>
> old part of line
> - PostgreSQL will accept a setting for any two-part parameter name.
>
> new part of line
> + PostgreSQL will accept a setting for any parameter name containing
> at least one dot.
>
> If I read new part of line just in context of custom options, it makes
> sense, but when I compare with old line I think below lines or variant
> of them might make more sense as this line is not restricted to just
> custom options:
>
> a. PostgreSQL will accept a setting for any parameter name containing
> two or more part parameter names.
> b. PostgreSQL will accept a setting for any parameter name containing
> one or more dots in parts of parameter names.
>
> It's just how user interpret any line, so may be your line is more
> meaningful to some users. If you don't think there is any need to
> change then keep as it is and let committer decide about it. I don't
> have any big problem with the current wording.
>
> I think Robert is still not fully convinced about this patch, but from
> my side review of patch with the current scope is complete, so I will
> mark it as Ready For Committer if nobody has objections for the same.

I had marked this patch as Ready For Committer, as I think in it's
current scope definition it's ready for next level of review.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

I think the idea that we should consider a different way of handling
tabular configuration data has merit. In fact, how much sense does it
make to have these options (the ones for which this patch is being
written) be GUCs in the first place? ALTER USER/DATABASE don't work for
them, they can't be usefully changed in the commandline, there's no
working SET.

If we had some way to plug these into pg_hba.conf parsing machinery
(which is tabular data), I would suggest that. But that doesn't sound
really sensible. I think the idea of putting this configuratio data
in a separate file, and perhaps a more convenient format than
three-level GUC options, should be considered.

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

On Mon, Sep 23, 2013 at 7:06 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> I think the idea that we should consider a different way of handling
> tabular configuration data has merit. In fact, how much sense does it
> make to have these options (the ones for which this patch is being
> written) be GUCs in the first place?

This is mainly for Customized option and or that it depends on
user, how he wants to name the variables.

> ALTER USER/DATABASE don't work for
> them, they can't be usefully changed in the commandline, there's no
> working SET.

Do you mean to say that it doesn't work for SET command?
As it is discussed upthread that if it works for
set_config()/SET,.. so it is better that it should be allowed through
postgresql.conf

> If we had some way to plug these into pg_hba.conf parsing machinery
> (which is tabular data), I would suggest that. But that doesn't sound
> really sensible. I think the idea of putting this configuratio data
> in a separate file, and perhaps a more convenient format than
> three-level GUC options, should be considered.

This will be really better if we can figure out a more sophisticated
way to handle, but I think if it currently works with other
mechanism's of GUC settings (set_config,SET, etc.), then what is the
harm in allowing it through postgresql.conf file?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Mon, Sep 23, 2013 at 9:36 AM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> I think the idea that we should consider a different way of handling
> tabular configuration data has merit. In fact, how much sense does it
> make to have these options (the ones for which this patch is being
> written) be GUCs in the first place? ALTER USER/DATABASE don't work for
> them, they can't be usefully changed in the commandline, there's no
> working SET.
>
> If we had some way to plug these into pg_hba.conf parsing machinery
> (which is tabular data), I would suggest that. But that doesn't sound
> really sensible. I think the idea of putting this configuratio data
> in a separate file, and perhaps a more convenient format than
> three-level GUC options, should be considered.

All very good points, IMHO. In a lot of cases, what you want is

sneazle.list='foo,bar'
sneazle.foo.prop1='zatz'
sneazle.bar.prop1='frotz'
etc.

But that means that the set of GUCs that exist to be SET needs to
change every time sneazle.list changes, and we haven't got any good
mechanism for that. I really think we're trying to squeeze a square
peg into a round hole here, and I accordingly propose to mark this
patch rejected.

It seems to me that if an extension wants to read and parse a
configuration file in $PGDATA, it doesn't need any special core
support for that. If there's enough consistency in terms of what
those configuration files look like across various extensions, we
might choose to provide a set of common tools in core to help parse
them. But I'm not too convinced any useful pattern will emerge.

Another option is to store the data in an actual table. One could
have sneazle.configtable='dbname.schemaname.tablename', for example.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi Robert,

On 2013-10-03 14:39:46 -0400, Robert Haas wrote:
> On Mon, Sep 23, 2013 at 9:36 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
> > I think the idea that we should consider a different way of handling
> > tabular configuration data has merit. In fact, how much sense does it
> > make to have these options (the ones for which this patch is being
> > written) be GUCs in the first place? ALTER USER/DATABASE don't work for
> > them, they can't be usefully changed in the commandline, there's no
> > working SET.
> >
> > If we had some way to plug these into pg_hba.conf parsing machinery
> > (which is tabular data), I would suggest that. But that doesn't sound
> > really sensible. I think the idea of putting this configuratio data
> > in a separate file, and perhaps a more convenient format than
> > three-level GUC options, should be considered.
>
> All very good points, IMHO. In a lot of cases, what you want is
>
> sneazle.list='foo,bar'
> sneazle.foo.prop1='zatz'
> sneazle.bar.prop1='frotz'
> etc.
>
> But that means that the set of GUCs that exist to be SET needs to
> change every time sneazle.list changes, and we haven't got any good
> mechanism for that.

It'd be pretty easy to have a function that removes everything inside a
certain namespace. That'd be enough to make EmitWarningsOnPlaceholders()
work, right?

> I really think we're trying to squeeze a square
> peg into a round hole here, and I accordingly propose to mark this
> patch rejected.

> It seems to me that if an extension wants to read and parse a
> configuration file in $PGDATA, it doesn't need any special core
> support for that. If there's enough consistency in terms of what
> those configuration files look like across various extensions, we
> might choose to provide a set of common tools in core to help parse
> them. But I'm not too convinced any useful pattern will emerge.

The problem is that such configuration formats needs to deal with a host
of already solved things like:
* triggering reload across backends
* presentation layer: SHOW/pg_settings
* The ability to override settings on certain levels: SET/ALTER
DATABASE/ALTER ...
* Correct handling of invalid values on reload and such
* parsed early enough to allocate shared memory

That's quite the truckload of things that need to be done by any new
format.

I don't really understand the resistance to the patch. It's a two line
change that doesn't allow anything that wasn't already allowed by other
means (SET, SELECT set_config(), -c). It sure isn't perfect for
everything but for some scenarios it improves the scenario sufficiently
that it'd make at least one extension author happy ;)

> Another option is to store the data in an actual table. One could
> have sneazle.configtable='dbname.schemaname.tablename', for example.

Doesn't really work if your extension needs to do stuff during startup
tho.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> It'd be pretty easy to have a function that removes everything inside a
> certain namespace. That'd be enough to make EmitWarningsOnPlaceholders()
> work, right?

Maybe, but I don't think you're going to paper over the problem that
easily. The GUC mechanism just isn't decided to support settings that
pop into and out of existence like that. It's not a coincidence that
there's no UNSET commands for GUCs. We have RESET but that means "go
back to the default", not "go away". You're trying to bend the
mechanism to do something that it fundamentally wasn't designed to do.
I don't think that's the right way to go, but if we do decide to go
in that direction it's going to take more than a trivial patch to get
there.

> The problem is that such configuration formats needs to deal with a host
> of already solved things like:
> * triggering reload across backends
> * presentation layer: SHOW/pg_settings
> * The ability to override settings on certain levels: SET/ALTER
> DATABASE/ALTER ...
> * Correct handling of invalid values on reload and such
> * parsed early enough to allocate shared memory
>
> That's quite the truckload of things that need to be done by any new
> format.

True.

> I don't really understand the resistance to the patch. It's a two line
> change that doesn't allow anything that wasn't already allowed by other
> means (SET, SELECT set_config(), -c). It sure isn't perfect for
> everything but for some scenarios it improves the scenario sufficiently
> that it'd make at least one extension author happy ;)

That's true, but I think the fact that those things work is an
oversight rather than a deliberate design decision.

>> Another option is to store the data in an actual table. One could
>> have sneazle.configtable='dbname.schemaname.tablename', for example.
>
> Doesn't really work if your extension needs to do stuff during startup
> tho.

Granted. There's not a perfect mechanism here. But I think we should
be devoting some thought to what a good mechanism that could be used
by core *and* contrib would look like, rather than thinking that a
quick hack is going to make the pain go away.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2013-10-04 09:57:41 -0400, Robert Haas wrote:
> On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > It'd be pretty easy to have a function that removes everything inside a
> > certain namespace. That'd be enough to make EmitWarningsOnPlaceholders()
> > work, right?
>
> Maybe, but I don't think you're going to paper over the problem that
> easily. The GUC mechanism just isn't decided to support settings that
> pop into and out of existence like that. It's not a coincidence that
> there's no UNSET commands for GUCs. We have RESET but that means "go
> back to the default", not "go away". You're trying to bend the
> mechanism to do something that it fundamentally wasn't designed to do.
> I don't think that's the right way to go, but if we do decide to go
> in that direction it's going to take more than a trivial patch to get
> there.

But that's not a new problem? It already exists and isn't really
excerbated by this.

> > I don't really understand the resistance to the patch. It's a two line
> > change that doesn't allow anything that wasn't already allowed by other
> > means (SET, SELECT set_config(), -c). It sure isn't perfect for
> > everything but for some scenarios it improves the scenario sufficiently
> > that it'd make at least one extension author happy ;)
>
> That's true, but I think the fact that those things work is an
> oversight rather than a deliberate design decision.

Yes, but it's already being used, so, while some minor restrictions
probably aren't to problematic, forbidding multiple dots outright seems
like unnecessarily breaking user applications.

> >> Another option is to store the data in an actual table. One could
> >> have sneazle.configtable='dbname.schemaname.tablename', for example.
> >
> > Doesn't really work if your extension needs to do stuff during startup
> > tho.
>
> Granted. There's not a perfect mechanism here. But I think we should
> be devoting some thought to what a good mechanism that could be used
> by core *and* contrib would look like, rather than thinking that a
> quick hack is going to make the pain go away.

I agree that we could use some more infrastructure around configuration,
but I fail to understand why it's this patch's duty to deliver it. And I
don't see why this patch would endanger any more groundbreaking
improvements.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Oct 4, 2013 at 10:14 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> But that's not a new problem? It already exists and isn't really
> excerbated by this.
...
> I agree that we could use some more infrastructure around configuration,
> but I fail to understand why it's this patch's duty to deliver it. And I
> don't see why this patch would endanger any more groundbreaking
> improvements.

You keep saying the ship has already sailed, but I think that's
inaccurate. IMHO, we haven't committed to anything in this area as a
matter of policy; I think the lack of a policy is demonstrated by the
inconsistencies you point out.

Now, if we are already committed to a policy of supporting the use
case you're targeting with this patch, then you're right: this is just
a trivial bug fix, and we ought to just take it for what it is and fix
whatever other issues come up later. But if we're not committed to
such a policy, then "support multi-level GUCs" is a new feature, and
it's entirely right to think that, just like any other new feature, it
needs to implement that feature completely rather than piecemeal. We
know from experience that when certain features (e.g. hash indexes)
are implemented incompletely, the resulting warts can remain behind
more or less indefinitely.

As I read the thread, Amit Kapila is in favor of your proposal. Pavel
Stehule, Alvaro Herrera, and I all questioned whether multi-level GUCs
were a good idea; at least 2 out of the 3 of us favor not committing
the patch out of uncertainty that we wish to be on the hook to support
such usages. Andrew Dunstan and Tom Lane agreed that the current
behavior was inconsistent but neither clearly endorsed relaxing the
checks in guc-file.l; in fact, Tom suggested tightening up SET
instead. Not one person argued that multi-level GUCs were already a
supported feature and that this patch was just plugging a gap in that
feature; the documentation also disagrees with that interpretation.
So I just don't think we have consensus that this is already the
policy or that it is a policy we should adopt.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2013-10-04 11:04:53 -0400, Robert Haas wrote:
> On Fri, Oct 4, 2013 at 10:14 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > But that's not a new problem? It already exists and isn't really
> > excerbated by this.
> ...
> > I agree that we could use some more infrastructure around configuration,
> > but I fail to understand why it's this patch's duty to deliver it. And I
> > don't see why this patch would endanger any more groundbreaking
> > improvements.
>
> You keep saying the ship has already sailed, but I think that's
> inaccurate. IMHO, we haven't committed to anything in this area as a
> matter of policy; I think the lack of a policy is demonstrated by the
> inconsistencies you point out.

Well, it is SQL exposed behaviour that exists that way since the initial
introduction of custom_variable_classes in 2004. Even if allowing
multiple levels wasn't originally intended, ISTM that thats a long time
to now declare it as a parsing bug. Especially as it doesn't actually
hurt.

> Now, if we are already committed to a policy of supporting the use
> case you're targeting with this patch, then you're right: this is just
> a trivial bug fix, and we ought to just take it for what it is and fix
> whatever other issues come up later. But if we're not committed to
> such a policy, then "support multi-level GUCs" is a new feature, and
> it's entirely right to think that, just like any other new feature, it
> needs to implement that feature completely rather than piecemeal. We
> know from experience that when certain features (e.g. hash indexes)
> are implemented incompletely, the resulting warts can remain behind
> more or less indefinitely.

Well, currently we're not committed to supporting it in postgresql.conf,
but I think there's little chance of removing it from SQL. So not
allowing it in the former doesn't buy us anything.

> As I read the thread, Amit Kapila is in favor of your proposal. Pavel
> Stehule, Alvaro Herrera, and I all questioned whether multi-level GUCs
> were a good idea; at least 2 out of the 3 of us favor not committing
> the patch out of uncertainty that we wish to be on the hook to support
> such usages. Andrew Dunstan and Tom Lane agreed that the current
> behavior was inconsistent but neither clearly endorsed relaxing the
> checks in guc-file.l; in fact, Tom suggested tightening up SET
> instead.

That's slightly different than how I read it ;). Tom seems to argue
against restricting SET further (28392(dot)1378476803(at)sss(dot)pgh(dot)pa(dot)us).

But yes, I certainly haven't convinced everyone.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services