Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-10-04 11:04:53 -0400, Robert Haas wrote: On Fri, Oct 4, 2013 at 10:14 AM, Andres Freund and...@2ndquadrant.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.1378476...@sss.pgh.pa.us). But yes, I certainly haven't convinced everyone. 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: [HACKERS] [RFC] Extend namespace of valid guc names
Hi Robert, On 2013-10-03 14:39:46 -0400, Robert Haas wrote: On Mon, Sep 23, 2013 at 9:36 AM, Alvaro Herrera alvhe...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-10-04 09:57:41 -0400, Robert Haas wrote: On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Oct 4, 2013 at 10:14 AM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Mon, Sep 23, 2013 at 9:36 AM, Alvaro Herrera alvhe...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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 Herrerahttp://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: [HACKERS] [RFC] Extend namespace of valid guc names
On Mon, Sep 23, 2013 at 7:06 PM, Alvaro Herrera alvhe...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Sep 20, 2013 at 9:48 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Thu, Sep 19, 2013 at 2:48 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Thu, Sep 19, 2013 at 6:19 PM, Andres Freund and...@2ndquadrant.com wrote:
On 2013-09-19 14:57:53 -0400, Robert Haas wrote:
On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund and...@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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
From 2ab86c1cd230b4187ee994447075bd6a72b408dc Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sat, 14 Sep 2013 23:15:10 +0200
Subject: [PATCH] Allow custom GUCs to be nested more than one level in config
files
Doing so was allowed via SET, postgres -c and set_config()
before. Allowing to do so is useful for grouping together variables
inside an extension's toplevel namespace.
There still are differences in the accepted variable names between the
different methods of setting GUCs after this commit, but the concensus
is that those are acceptable.
---
doc/src/sgml/config.sgml | 29 -
src/backend/utils/misc/guc-file.l | 2 +-
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 370aa09..ae9ef3b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6375,21 +6375,32 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
/para
para
- Custom options have two-part names: an extension name, then a dot, then
- the parameter name proper, much like qualified names in SQL. An example
- is literalplpgsql.variable_conflict/.
+ Custom options consist out of an extension name, then a dot, zero
+ or more extension internal namespaces each with a trailing dot,
+ then the parameter name proper, much like qualified names in
+ SQL. An example without extension internal namespaces
+ is literalplpgsql.variable_conflict/, one with
+ is literalfoo.a.connection/.
/para
para
Because custom options may need to be set in processes that have not
loaded the relevant extension module, productnamePostgreSQL/
- will accept a setting for any two-part parameter name. Such variables
- are treated as placeholders and have no function until the module that
- defines them is loaded. When an extension module is loaded, it will add
- its variable definitions, convert any placeholder values according to
- those definitions, and issue warnings for any unrecognized placeholders
- that begin with its extension name.
+ will accept a setting for any parameter name containing at least one dot.
+ Such variables are treated as placeholders and have no function until the
+ module that defines them is loaded. When an extension module is loaded,
+ it will add its variable definitions, convert any placeholder values
+ according to those definitions, and issue warnings for any unrecognized
+ placeholders that begin with its extension name.
/para
+
+para
+ Note that options set in filenamepostgresql.conf/ and files
+ it includes have different restrictions than names set
+ with commandSET/, commandSELECT set_config(...)/, or ones
+ passed directly to commandpostgres/ and commandpg_ctl/.
+/para
+
/sect1
sect1 id=runtime-config-developer
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index b730a12..ff4202d 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -78,7 +78,7 @@ LETTER [A-Za-z_\200-\377]
LETTER_OR_DIGIT [A-Za-z_0-9\200-\377]
ID{LETTER}{LETTER_OR_DIGIT}*
-QUALIFIED_ID {ID}.{ID}
+QUALIFIED_ID {ID}(.{ID})+
UNQUOTED_STRING {LETTER}({LETTER_OR_DIGIT}|[-._:/])*
STRING \'([^'\\\n]|\\.|\'\')*\'
--
1.8.4.21.g992c386.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-09-19 14:57:53 -0400, Robert Haas wrote:
On Tue, Sep 17, 2013 at 6:56 PM, Andres Freund and...@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Thu, Sep 19, 2013 at 2:48 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-09-17 10:23:30 -0400, Robert Haas wrote: On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-17 10:23:30 -0400, Robert Haas wrote: On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Hi, On 2013-09-17 16:24:34 -0400, Robert Haas wrote: On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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
From 9fa69ccc79c7bc42be554bc3f4e740c4e77d50d7 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sat, 14 Sep 2013 23:15:10 +0200
Subject: [PATCH] Allow custom GUCs to be nested more than one level in config
files
Doing so was allowed via SET, postgres -c and set_config()
before. Allowing to do so is useful for grouping together variables
inside an extension's toplevel namespace.
There still are differences in the accepted variable names between the
different methods of setting GUCs after this commit, but the concensus
is that those are acceptable.
---
src/backend/utils/misc/guc-file.l | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index b730a12..ff4202d 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -78,7 +78,7 @@ LETTER [A-Za-z_\200-\377]
LETTER_OR_DIGIT [A-Za-z_0-9\200-\377]
ID{LETTER}{LETTER_OR_DIGIT}*
-QUALIFIED_ID {ID}.{ID}
+QUALIFIED_ID {ID}(.{ID})+
UNQUOTED_STRING {LETTER}({LETTER_OR_DIGIT}|[-._:/])*
STRING \'([^'\\\n]|\\.|\'\')*\'
--
1.8.4.21.g992c386.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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.1barblub = 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-09-06 10:13:23 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.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.1barblub = 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Andres Freund and...@2ndquadrant.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.1barblub = 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 09/06/2013 08:48 PM, Tom Lane wrote: Andres Freund and...@2ndquadrant.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Ü -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Hannu Krosing ha...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-09-06 14:48:33 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Sep 6, 2013 at 6:31 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-06 14:48:33 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Robert Haas robertmh...@gmail.com writes: On Fri, Sep 6, 2013 at 6:31 PM, Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-02-25 21:13:25 -0500, Tom Lane wrote:
Andres Freund and...@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [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
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index d3565a5..d9ef10e 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -78,7 +78,8 @@ LETTER [A-Za-z_\200-\377]
LETTER_OR_DIGIT [A-Za-z_0-9\200-\377]
ID{LETTER}{LETTER_OR_DIGIT}*
-QUALIFIED_ID {ID}.{ID}
+SECONDARY_ID {LETTER_OR_DIGIT}+
+QUALIFIED_ID {ID}(.{SECONDARY_ID})+
UNQUOTED_STRING {LETTER}({LETTER_OR_DIGIT}|[-._:/])*
STRING \'([^'\\\n]|\\.|\'\')*\'
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Hello Why? There are no multilevels structures in pg. Variables should be joined with schemas or extensions. Other levels are messy.
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Andres Freund and...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
