another idea for changing global configuration settings from SQL
Independent of the discussion of how to edit configuration files from
SQL, I had another idea how many of the use cases for this could be handled.
We already have the ability to store in pg_db_role_setting configuration
settings for
specific user, specific database
specific user, any database
any user, specific database
The existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.
There would also be the advantage that pg_dumpall would save these settings.
Thoughts?
On Thu, Nov 15, 2012 at 12:53 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Independent of the discussion of how to edit configuration files from
SQL, I had another idea how many of the use cases for this could be handled.We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.There would also be the advantage that pg_dumpall would save these settings.
Thoughts?
Personally, I think that would be wonderful.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Le jeudi 15 novembre 2012 18:53:15, Peter Eisentraut a écrit :
Independent of the discussion of how to edit configuration files from
SQL, I had another idea how many of the use cases for this could be
handled.We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.There would also be the advantage that pg_dumpall would save these
settings.Thoughts?
I like the idea.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.There would also be the advantage that pg_dumpall would save these settings.
I think this is a great idea.
One caveat: we really, really, really need a system view which allows
DBAs to easily review settings defined for specific users and databases.
Right now, it requires significant pg_catalog hacking expertise to pull
out user-specific settings.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Peter Eisentraut <peter_e@gmx.net> writes:
The existing infrastructure would also support
any user, any database (= all the time)There would also be the advantage that pg_dumpall would save these settings.
Thoughts?
That's brilliant. +1.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Thu, Nov 15, 2012 at 6:53 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Independent of the discussion of how to edit configuration files from
SQL, I had another idea how many of the use cases for this could be handled.We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.
How hard would it be to make it work for SIGHUP? I can see how it
would be impossible to handle things like POSTMASTER, but SIGHUP seems
like it should be doable somehow?
There would also be the advantage that pg_dumpall would save these settings.
Thoughts?
I like it. Not as a replacement for the other facility, but as another
way of doing it. And I'd expect it could be the "main way" for manual
changes, but tools would still need access to the other way of course.
We probably need to enhance pg_settings to tell the user *where* the
setting came from whe nit's set this way. In fact, we need this
already, since it can be hard to track down...
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Nov 15, 2012 at 6:53 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.
How hard would it be to make it work for SIGHUP?
One issue is that pg_db_role_setting is currently considered only at
session start, and unless you want to hack that somehow, these new
settings would only be absorbed by freshly-started sessions.
Now, there's already a good deal of asynchrony in when individual
processes notice postgresql.conf updates, whether they're for SIGHUP
or lesser settings. So maybe that's all right. If you weren't happy
about it, one of several things you'd have to work out is how to send a
SIGHUP only after you've committed the changes.
Another and probably bigger thing is that SIGHUP is used for settings
that do something useful only in background processes (eg checkpointer).
Some of those processes are not capable of reading system catalogs at
all. This is particularly a showstopper for settings affecting the
postmaster itself, which is most certainly *not* going to grow the
ability to read catalogs.
On the whole I suspect the existing push towards rewritable config file
entries is going to go further in less time for anything whose effects
aren't limited to regular backend sessions. I don't object to Peter's
idea as such, but it's not going to help us for SIGHUP settings.
regards, tom lane
On 11/16/2012 02:38 AM, Josh Berkus wrote:
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.There would also be the advantage that pg_dumpall would save these settings.
I think this is a great idea.
One caveat: we really, really, really need a system view which allows
DBAs to easily review settings defined for specific users and databases.
Right now, it requires significant pg_catalog hacking expertise to pull
out user-specific settings.
A system information function like settings_for_user('username') would
certainly be welcome, showing:
setting_name setting_value setting_origin
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 11/15/2012 11:38 PM, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Nov 15, 2012 at 6:53 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.How hard would it be to make it work for SIGHUP?
One issue is that pg_db_role_setting is currently considered only at
session start, and unless you want to hack that somehow, these new
settings would only be absorbed by freshly-started sessions.Now, there's already a good deal of asynchrony in when individual
processes notice postgresql.conf updates, whether they're for SIGHUP
or lesser settings. So maybe that's all right. If you weren't happy
about it, one of several things you'd have to work out is how to send a
SIGHUP only after you've committed the changes.Another and probably bigger thing is that SIGHUP is used for settings
that do something useful only in background processes (eg checkpointer).
Some of those processes are not capable of reading system catalogs at
all. This is particularly a showstopper for settings affecting the
postmaster itself, which is most certainly *not* going to grow the
ability to read catalogs.On the whole I suspect the existing push towards rewritable config file
entries is going to go further in less time for anything whose effects
aren't limited to regular backend sessions. I don't object to Peter's
idea as such, but it's not going to help us for SIGHUP settings.regards, tom lane
Why not just make the sending SIGHUP a separate command as it is now ?
SELECT pg_reload_config();
Hannu
On 16-11-2012 12:27, Hannu Krosing wrote:
Why not just make the sending SIGHUP a separate command as it is now ?
SELECT pg_reload_config();
... or even a RELOAD command. I've already coded a WIP patch for such command.
--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Attachments:
reload.patchtext/x-patch; name=reload.patchDownload
diff --git a/doc/src/sgml/ref/allfiles.sgml b/doc/src/sgml/ref/allfiles.sgml
index df84054..f7abb57 100644
--- a/doc/src/sgml/ref/allfiles.sgml
+++ b/doc/src/sgml/ref/allfiles.sgml
@@ -137,6 +137,7 @@ Complete list of usable sgml source files in this directory.
<!ENTITY reassignOwned SYSTEM "reassign_owned.sgml">
<!ENTITY reindex SYSTEM "reindex.sgml">
<!ENTITY releaseSavepoint SYSTEM "release_savepoint.sgml">
+<!ENTITY reload SYSTEM "reload.sgml">
<!ENTITY reset SYSTEM "reset.sgml">
<!ENTITY revoke SYSTEM "revoke.sgml">
<!ENTITY rollback SYSTEM "rollback.sgml">
diff --git a/doc/src/sgml/ref/reload.sgml b/doc/src/sgml/ref/reload.sgml
new file mode 100644
index 0000000..42baab9
--- /dev/null
+++ b/doc/src/sgml/ref/reload.sgml
@@ -0,0 +1,51 @@
+<!-- doc/src/sgml/ref/reload.sgml -->
+
+<refentry id="sql-reload">
+ <refmeta>
+ <refentrytitle>RELOAD</refentrytitle>
+ <manvolnum>7</manvolnum>
+ <refmiscinfo>SQL - Language Statements</refmiscinfo>
+ </refmeta>
+
+ <refnamediv>
+ <refname>RELOAD</refname>
+ <refpurpose>reread configuration files</refpurpose>
+ </refnamediv>
+
+ <indexterm zone="sql-reload">
+ <primary>RELOAD</primary>
+ </indexterm>
+
+ <refsynopsisdiv>
+<synopsis>
+RELOAD
+</synopsis>
+ </refsynopsisdiv>
+
+ <refsect1>
+ <title>Description</title>
+
+ <para>
+ The <command>RELOAD</command> command triggers <command>postgres</command> to
+ reread its configuration files (<filename>postgresql.conf</filename>,
+ <filename>pg_hba.conf</filename>, etc.). This allows changing of
+ configuration-file options that do not require a complete restart to take
+ effect. <command>RELOAD</command> command has the same behavior as
+ <command>pg_ctl</command> <option>reload</option> option (see <xref
+ linkend="app-pg-ctl">).
+ </para>
+
+ <para>
+ Only superusers can call <command>RELOAD</command>.
+ </para>
+ </refsect1>
+
+ <refsect1>
+ <title>Compatibility</title>
+
+ <para>
+ The <command>RELOAD</command> command is a
+ <productname>PostgreSQL</productname> language extension.
+ </para>
+ </refsect1>
+</refentry>
diff --git a/doc/src/sgml/reference.sgml b/doc/src/sgml/reference.sgml
index 0872168..ae5fb67 100644
--- a/doc/src/sgml/reference.sgml
+++ b/doc/src/sgml/reference.sgml
@@ -165,6 +165,7 @@
&reassignOwned;
&reindex;
&releaseSavepoint;
+ &reload;
&reset;
&revoke;
&rollback;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 9387ee9..5dd6d0e 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4394,6 +4394,9 @@ copyObject(const void *from)
case T_CheckPointStmt:
retval = (void *) makeNode(CheckPointStmt);
break;
+ case T_ReloadStmt:
+ retval = (void *) makeNode(ReloadStmt);
+ break;
case T_CreateSchemaStmt:
retval = _copyCreateSchemaStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 95a95f4..7118bb6 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2881,6 +2881,9 @@ equal(const void *a, const void *b)
case T_CheckPointStmt:
retval = true;
break;
+ case T_ReloadStmt:
+ retval = true;
+ break;
case T_CreateSchemaStmt:
retval = _equalCreateSchemaStmt(a, b);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e4ff76e..b853e8f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -245,7 +245,7 @@ static void processCASbits(int cas_bits, int location, const char *constrType,
VariableResetStmt VariableSetStmt VariableShowStmt
ViewStmt CheckPointStmt CreateConversionStmt
DeallocateStmt PrepareStmt ExecuteStmt
- DropOwnedStmt ReassignOwnedStmt
+ DropOwnedStmt ReassignOwnedStmt ReloadStmt
AlterTSConfigurationStmt AlterTSDictionaryStmt
%type <node> select_no_parens select_with_parens select_clause
@@ -571,7 +571,7 @@ static void processCASbits(int cas_bits, int location, const char *constrType,
QUOTE
RANGE READ REAL REASSIGN RECHECK RECURSIVE REF REFERENCES REINDEX
- RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA
+ RELATIVE_P RELEASE RELOAD RENAME REPEATABLE REPLACE REPLICA
RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK
ROW ROWS RULE
@@ -794,6 +794,7 @@ stmt :
| PrepareStmt
| ReassignOwnedStmt
| ReindexStmt
+ | ReloadStmt
| RemoveAggrStmt
| RemoveFuncStmt
| RemoveOperStmt
@@ -1657,6 +1658,20 @@ CheckPointStmt:
}
;
+/*****************************************************************************
+ *
+ * RELOAD
+ *
+ *****************************************************************************/
+
+ReloadStmt:
+ RELOAD
+ {
+ ReloadStmt *n = makeNode(ReloadStmt);
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
@@ -12605,6 +12620,7 @@ unreserved_keyword:
| REINDEX
| RELATIVE_P
| RELEASE
+ | RELOAD
| RENAME
| REPEATABLE
| REPLACE
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b223fee..a0e55df 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2116,6 +2116,15 @@ reset_shared(int port)
CreateSharedMemoryAndSemaphores(false, port);
}
+/*
+ * RELOAD command -- rereads configuration files
+ */
+void
+ReloadConfiguration(void)
+{
+ signal_child(PostmasterPid, SIGHUP);
+}
+
/*
* SIGHUP -- reread config files, and tell children to do same
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 491bd29..5e0f9c9 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -54,6 +54,7 @@
#include "miscadmin.h"
#include "parser/parse_utilcmd.h"
#include "postmaster/bgwriter.h"
+#include "postmaster/postmaster.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteRemove.h"
#include "storage/fd.h"
@@ -1258,6 +1259,15 @@ standard_ProcessUtility(Node *parsetree,
(RecoveryInProgress() ? 0 : CHECKPOINT_FORCE));
break;
+ case T_ReloadStmt:
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to do RELOAD")));
+
+ ReloadConfiguration();
+ break;
+
case T_ReindexStmt:
{
ReindexStmt *stmt = (ReindexStmt *) parsetree;
@@ -2190,6 +2200,10 @@ CreateCommandTag(Node *parsetree)
tag = "CHECKPOINT";
break;
+ case T_ReloadStmt:
+ tag = "RELOAD";
+ break;
+
case T_ReindexStmt:
tag = "REINDEX";
break;
@@ -2697,6 +2711,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_ALL;
break;
+ case T_ReloadStmt:
+ lev = LOGSTMT_ALL;
+ break;
+
case T_ReindexStmt:
lev = LOGSTMT_ALL; /* should this be DDL? */
break;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 438a1d9..d9331ad 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -361,6 +361,7 @@ typedef enum NodeTag
T_AlterExtensionContentsStmt,
T_CreateEventTrigStmt,
T_AlterEventTrigStmt,
+ T_ReloadStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 8834499..15d400a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2459,6 +2459,15 @@ typedef struct CheckPointStmt
} CheckPointStmt;
/* ----------------------
+ * Reload Statement
+ * ----------------------
+ */
+typedef struct ReloadStmt
+{
+ NodeTag type;
+} ReloadStmt;
+
+/* ----------------------
* Discard Statement
* ----------------------
*/
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index af60dac..389f201 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -304,6 +304,7 @@ PG_KEYWORD("references", REFERENCES, RESERVED_KEYWORD)
PG_KEYWORD("reindex", REINDEX, UNRESERVED_KEYWORD)
PG_KEYWORD("relative", RELATIVE_P, UNRESERVED_KEYWORD)
PG_KEYWORD("release", RELEASE, UNRESERVED_KEYWORD)
+PG_KEYWORD("reload", RELOAD, UNRESERVED_KEYWORD)
PG_KEYWORD("rename", RENAME, UNRESERVED_KEYWORD)
PG_KEYWORD("repeatable", REPEATABLE, UNRESERVED_KEYWORD)
PG_KEYWORD("replace", REPLACE, UNRESERVED_KEYWORD)
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 0fe7ec2..444f72e 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -49,6 +49,8 @@ extern const char *progname;
extern void PostmasterMain(int argc, char *argv[]) __attribute__((noreturn));
extern void ClosePostmasterPorts(bool am_syslogger);
+extern void ReloadConfiguration(void);
+
extern int MaxLivePostmasterChildren(void);
#ifdef EXEC_BACKEND
On 11/15/12 12:53 PM, Peter Eisentraut wrote:
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
Alright, any suggestions for the syntax? We currently have
ALTER DATABASE ... SET ...
ALTER ROLE ... SET ...
ALTER ROLE ... IN DATABASE ... SET
I was thinking something like
ALTER ROLE ANY SET ...
in order to avoid creating a new top-level command, but it's not pretty.
Another way might be something like
SET GLOBAL name = value
but that would make the command very dissimilar from the other ones,
even though their effects are closely related.
On 16-11-2012 12:59, Peter Eisentraut wrote:
Another way might be something like
SET GLOBAL name = value
That's the exact syntax I'm about to propose for this feature (changing
settings using SQL).
Are you thinking about allowing changing all configuration settings or just a
subset of it? As said by others, using pg_db_role_setting only works for
sighup, superuser, and user context. How would you solve the backend and
postmaster context?
--
Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Peter Eisentraut <peter_e@gmx.net> writes:
Another way might be something like
SET GLOBAL name = value
but that would make the command very dissimilar from the other ones,
even though their effects are closely related.
Yeah. I think it would also give people a wrong impression about when
the setting would take effect, because existing variants of SET are
immediate (for some value of immediate). And it would invite confusion
with the write-the-config-file patch, which is going to end up using
some syntax much like this one. I think we really want to use ALTER,
though I agree none of the alternatives are great.
Have you considered ALTER SYSTEM SET ... ? We'd talked about that in
the context of the other patch, but it seems to fit much more naturally
with this one. Or maybe ALTER GLOBAL SET or ALTER ALL SET.
regards, tom lane
On Thu, Nov 15, 2012 at 5:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Another and probably bigger thing is that SIGHUP is used for settings
that do something useful only in background processes (eg checkpointer).
Some of those processes are not capable of reading system catalogs at
all. This is particularly a showstopper for settings affecting the
postmaster itself, which is most certainly *not* going to grow the
ability to read catalogs.
This seems like a pretty large strike against this whole idea. In
fact, I think we might want to abandon this whole approach on this
basis.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 11/16/2012 06:05 PM, Robert Haas wrote:
On Thu, Nov 15, 2012 at 5:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Another and probably bigger thing is that SIGHUP is used for settings
that do something useful only in background processes (eg checkpointer).
Some of those processes are not capable of reading system catalogs at
all. This is particularly a showstopper for settings affecting the
postmaster itself, which is most certainly *not* going to grow the
ability to read catalogs.This seems like a pretty large strike against this whole idea. In
fact, I think we might want to abandon this whole approach on this
basis.
Can't we keep a separate text .conf file specifically for the background
processes which can't read system catalogs. It could contain only the
GUCs these processes are interested in.
This file can be written out via a OnCommit hook which unhooks itself
when the work is done.
This approach should guarantee that the latest committed state is
always in the text file.
Hannu
Hannu Krosing <hannu@krosing.net> writes:
Can't we keep a separate text .conf file specifically for the background
processes which can't read system catalogs. It could contain only the
GUCs these processes are interested in.
What's the value of that, compared to the existing proposal for
write-a-text-file-directly? It seems like useless complication.
If we could move *all* the GUCs into system catalogs, maybe it'd be
worth the trouble, but I think that's a fundamentally bad idea.
It will make it impossible to change settings when the system is down,
and thus for example impossible to fix configuration errors that
prevent the postmaster from starting. I think we should stick with
the principle that the text file is the primary authority, and that
means we don't need a system catalog entry for global settings.
A possibly instructive precedent is that we got rid of
pg_tablespace.spclocation after deciding it was counterproductive
to have a catalog entry that wasn't the authoritative state.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes:
Have you considered ALTER SYSTEM SET ... ? We'd talked about that in
the context of the other patch, but it seems to fit much more naturally
with this one. Or maybe ALTER GLOBAL SET or ALTER ALL SET.
I would paint that one ALTER SYSTEM SET and the file based one ALTER
CONFIGURATION SET. No new keyword were armed in that proposal.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Saturday, November 17, 2012 3:35 AM Dimitri Fontaine wrote:
Tom Lane <tgl@sss.pgh.pa.us> writes:
Have you considered ALTER SYSTEM SET ... ? We'd talked about that in
the context of the other patch, but it seems to fit much morenaturally
with this one. Or maybe ALTER GLOBAL SET or ALTER ALL SET.
I would paint that one ALTER SYSTEM SET and the file based one ALTER
CONFIGURATION SET. No new keyword were armed in that proposal.
One more could be to have built-in function
pg_change_config(level,key,value)
level - PG_NEW_CONNECTION
- PG_SYTEM_LEVEL
Level will distinguish how and when the value will be used.
With Regards,
Amit Kapila.
On Fri, Nov 16, 2012 at 2:53 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
Independent of the discussion of how to edit configuration files from
SQL, I had another idea how many of the use cases for this could be handled.We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
The only thing you couldn't handle that way are SIGHUP settings, but the
often-cited use cases work_mem, logging, etc. would work.There would also be the advantage that pg_dumpall would save these settings.
Thoughts?
In this approach, we cannot change the settings in the standby?
If yes, I don't like this approach.
Regards,
--
Fujii Masao
On 11/15/12 12:53 PM, Peter Eisentraut wrote:
We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
Here is a patch for that.
The internals are straightforward. Actually, we might want to refactor
this a bit later, unifying the AlterRoleSet and AlterDatabaseSet parse
nodes and the functions that do the work, because it's really all the same.
The SQL level interface is a bit odd. The existing facilities are
ALTER ROLE / SET
ALTER DATABASE / SET
ALTER ROLE / IN DATABASE / SET
Since the original design somehow considered roles to be superior to
databases in this regard, I added the global setting as ALTER ROLE ALL
SET ..., but that's obviously arbitrary. Most other variants would
probably be much more invasive, though.
Attachments:
pg-alter-role-all-set.patchtext/plain; charset=UTF-8; name=pg-alter-role-all-set.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index 2fbba53..6fa51ee 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -39,9 +39,9 @@
ALTER ROLE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
ALTER ROLE <replaceable class="PARAMETER">name</replaceable> [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] SET <replaceable>configuration_parameter</replaceable> { TO | = } { <replaceable>value</replaceable> | DEFAULT }
-ALTER ROLE <replaceable class="PARAMETER">name</replaceable> [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] SET <replaceable>configuration_parameter</replaceable> FROM CURRENT
-ALTER ROLE <replaceable class="PARAMETER">name</replaceable> [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] RESET <replaceable>configuration_parameter</replaceable>
-ALTER ROLE <replaceable class="PARAMETER">name</replaceable> [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] RESET ALL
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL ] [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] SET <replaceable>configuration_parameter</replaceable> FROM CURRENT
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL ] [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] RESET <replaceable>configuration_parameter</replaceable>
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL ] [ IN DATABASE <replaceable class="PARAMETER">database_name</replaceable> ] RESET ALL
</synopsis>
</refsynopsisdiv>
@@ -83,8 +83,15 @@ <title>Description</title>
<para>
The remaining variants change a role's session default for a configuration
variable, either for all databases or, when the <literal>IN
- DATABASE</literal> clause is specified, only for sessions in
- the named database. Whenever the role subsequently
+ DATABASE</literal> clause is specified, only for sessions in the named
+ database. If <literal>ALL</literal> is specified instead of a role name,
+ this changes the setting for all roles. Using <literal>ALL</literal>
+ with <literal>IN DATABASE</literal> is effectively the same as using the
+ command <literal>ALTER DATABASE ... SET ...</literal>.
+ </para>
+
+ <para>
+ Whenever the role subsequently
starts a new session, the specified value becomes the session
default, overriding whatever setting is present in
<filename>postgresql.conf</> or has been received from the <command>postgres</command>
@@ -93,12 +100,17 @@ <title>Description</title>
<xref linkend="sql-set-session-authorization"> does not cause new
configuration values to be set.
Settings set for all databases are overridden by database-specific settings
- attached to a role.
+ attached to a role. Settings for specific databases or specific roles override
+ settings for all roles.
+ </para>
+
+ <para>
Superusers can change anyone's session defaults. Roles having
<literal>CREATEROLE</> privilege can change defaults for non-superuser
roles. Ordinary roles can only set defaults for themselves.
Certain configuration variables cannot be set this way, or can only be
- set if a superuser issues the command.
+ set if a superuser issues the command. Only superusers can change a setting
+ for all roles in all databases.
</para>
</refsect1>
@@ -307,6 +319,7 @@ <title>See Also</title>
<simplelist type="inline">
<member><xref linkend="sql-createrole"></member>
<member><xref linkend="sql-droprole"></member>
+ <member><xref linkend="sql-alterdatabase"></member>
<member><xref linkend="sql-set"></member>
</simplelist>
</refsect1>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 3ba877d..5edb59a 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -814,41 +814,46 @@ static void DelRoleMems(const char *rolename, Oid roleid,
{
HeapTuple roletuple;
Oid databaseid = InvalidOid;
- Oid roleid;
+ Oid roleid = InvalidOid;
- roletuple = SearchSysCache1(AUTHNAME, PointerGetDatum(stmt->role));
+ if (stmt->role)
+ {
+ roletuple = SearchSysCache1(AUTHNAME, PointerGetDatum(stmt->role));
- if (!HeapTupleIsValid(roletuple))
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("role \"%s\" does not exist", stmt->role)));
+ if (!HeapTupleIsValid(roletuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("role \"%s\" does not exist", stmt->role)));
- roleid = HeapTupleGetOid(roletuple);
+ roleid = HeapTupleGetOid(roletuple);
- /*
- * Obtain a lock on the role and make sure it didn't go away in the
- * meantime.
- */
- shdepLockAndCheckObject(AuthIdRelationId, HeapTupleGetOid(roletuple));
+ /*
+ * Obtain a lock on the role and make sure it didn't go away in the
+ * meantime.
+ */
+ shdepLockAndCheckObject(AuthIdRelationId, HeapTupleGetOid(roletuple));
- /*
- * To mess with a superuser you gotta be superuser; else you need
- * createrole, or just want to change your own settings
- */
- if (((Form_pg_authid) GETSTRUCT(roletuple))->rolsuper)
- {
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to alter superusers")));
- }
- else
- {
- if (!have_createrole_privilege() &&
- HeapTupleGetOid(roletuple) != GetUserId())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied")));
+ /*
+ * To mess with a superuser you gotta be superuser; else you need
+ * createrole, or just want to change your own settings
+ */
+ if (((Form_pg_authid) GETSTRUCT(roletuple))->rolsuper)
+ {
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter superusers")));
+ }
+ else
+ {
+ if (!have_createrole_privilege() &&
+ HeapTupleGetOid(roletuple) != GetUserId())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied")));
+ }
+
+ ReleaseSysCache(roletuple);
}
/* look up and lock the database, if specified */
@@ -856,10 +861,29 @@ static void DelRoleMems(const char *rolename, Oid roleid,
{
databaseid = get_database_oid(stmt->database, false);
shdepLockAndCheckObject(DatabaseRelationId, databaseid);
+
+ if (!stmt->role)
+ {
+ /*
+ * If no role is specified, then this is effectively the same as
+ * ALTER DATABASE ... SET, so use the same permission check.
+ */
+ if (!pg_database_ownercheck(databaseid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE,
+ stmt->database);
+ }
+ }
+
+ if (!stmt->role && !stmt->database)
+ {
+ /* Must be superuser to alter settings globally. */
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter settings globally")));
}
- AlterSetting(databaseid, HeapTupleGetOid(roletuple), stmt->setstmt);
- ReleaseSysCache(roletuple);
+ AlterSetting(databaseid, roleid, stmt->setstmt);
return roleid;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 76ef11e..fab330d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1018,6 +1018,14 @@ AlterRoleSetStmt:
n->setstmt = $5;
$$ = (Node *)n;
}
+ | ALTER ROLE ALL opt_in_database SetResetClause
+ {
+ AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
+ n->role = NULL;
+ n->database = $4;
+ n->setstmt = $5;
+ $$ = (Node *)n;
+ }
;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 7e21cea..8427006 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -1010,6 +1010,7 @@
ApplySetting(databaseid, roleid, relsetting, PGC_S_DATABASE_USER);
ApplySetting(InvalidOid, roleid, relsetting, PGC_S_USER);
ApplySetting(databaseid, InvalidOid, relsetting, PGC_S_DATABASE);
+ ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_GLOBAL);
heap_close(relsetting, AccessShareLock);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ac5e4f3..0c6ab9b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -500,6 +500,7 @@ static char *config_enum_get_options(struct config_enum * record,
/* PGC_S_ENV_VAR */ "environment variable",
/* PGC_S_FILE */ "configuration file",
/* PGC_S_ARGV */ "command line",
+ /* PGC_S_GLOBAL */ "global",
/* PGC_S_DATABASE */ "database",
/* PGC_S_USER */ "user",
/* PGC_S_DATABASE_USER */ "database user",
@@ -5149,7 +5150,7 @@ struct config_generic **
*/
elevel = IsUnderPostmaster ? DEBUG3 : LOG;
}
- else if (source == PGC_S_DATABASE || source == PGC_S_USER ||
+ else if (source == PGC_S_GLOBAL || source == PGC_S_DATABASE || source == PGC_S_USER ||
source == PGC_S_DATABASE_USER)
elevel = WARNING;
else
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 0023c00..d497b1f 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -87,6 +87,7 @@ typedef enum
PGC_S_ENV_VAR, /* postmaster environment variable */
PGC_S_FILE, /* postgresql.conf */
PGC_S_ARGV, /* postmaster command line */
+ PGC_S_GLOBAL, /* global in-database setting */
PGC_S_DATABASE, /* per-database setting */
PGC_S_USER, /* per-user setting */
PGC_S_DATABASE_USER, /* per-user-and-database setting */
Hello Peter
I am looking on your patch.
I found only one issue
in documentation is role name or keyword ALL marked as optional, but
it is mandatory
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL
] [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] SET
<replaceable>configuration_parameter</replaceable> FROM CURRENT
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL
] [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] RESET
<replaceable>configuration_parameter</replaceable>
+ALTER ROLE [ <replaceable class="PARAMETER">name</replaceable> | ALL
] [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] RESET ALLshould be
+ALTER ROLE { <replaceable class="PARAMETER">name</replaceable> | ALL
} [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] SET
<replaceable>configuration_parameter</replaceable> FROM CURRENT
+ALTER ROLE { <replaceable class="PARAMETER">name</replaceable> | ALL
} [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] RESET
<replaceable>configuration_parameter</replaceable>
+ALTER ROLE { <replaceable class="PARAMETER">name</replaceable> | ALL
} [ IN DATABASE <replaceable
class="PARAMETER">database_name</replaceable> ] RESET ALL???
Regards
Pavel Stehule
2013/1/15 Peter Eisentraut <peter_e@gmx.net>:
On 11/15/12 12:53 PM, Peter Eisentraut wrote:
We already have the ability to store in pg_db_role_setting configuration
settings forspecific user, specific database
specific user, any database
any user, specific databaseThe existing infrastructure would also support
any user, any database (= all the time)
All you'd need is to add
ApplySetting(InvalidOid, InvalidOid, relsetting, PGC_S_$SOMETHING);
in postinit.c, and have some SQL command to modify this setting.
Here is a patch for that.
The internals are straightforward. Actually, we might want to refactor
this a bit later, unifying the AlterRoleSet and AlterDatabaseSet parse
nodes and the functions that do the work, because it's really all the same.The SQL level interface is a bit odd. The existing facilities are
ALTER ROLE / SET
ALTER DATABASE / SET
ALTER ROLE / IN DATABASE / SETSince the original design somehow considered roles to be superior to
databases in this regard, I added the global setting as ALTER ROLE ALL
SET ..., but that's obviously arbitrary. Most other variants would
probably be much more invasive, though.--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers