Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
--On 1. Oktober 2009 17:22:06 -0400 Alvaro Herrera wrote: - The patch as is has still some locking problems (AlterRoleSet() has a XXX about that): I've managed to create dead entries for a role or a database in pg_db_role_setting while altering and dropping a role/database in two concurrent sessions. Yeah, I was playing with that too. I think we need a few extra LockSharedObject calls, and not only in the new code :-( (This troubles me in the case of databases, because we already grab a lock on it during connection establishing, so this could cause extra contention there.) I have marked the patch as "Ready For Committer", so it can be taken by a committer to help to resolve the remaining locking issue. There seems no other issues left. If this is too hasty, i can set it back to whatever you think its appropriate. -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
--On 1. Oktober 2009 17:22:06 -0400 Alvaro Herrera wrote: - ALTER ROLE ... IN DATABASE is missing some documentation. If you want, i can work on this. Please. Here's a patch for this. I've kept it separately, so it's easier for you to merge it into your version. -- Thanks Bernd alter_role_docs.patch Description: Binary data -- 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] TODO item: Allow more complex user/database default GUC settings
Bernd Helmle escribió: > > > --On 30. September 2009 13:19:53 -0400 Alvaro Herrera > wrote: > > >>I think it would be helpful if you could post ONE patch with all the > >>changes and all the new files in the diff. AIUI, "the" patch is now > >>split across three separate emails. :-( > > > >That's correct, here it is. > > Some additional notes: > > - ALTER ROLE ... IN DATABASE is missing some documentation. If you > want, i can work on this. Please. > - The patch as is has still some locking problems (AlterRoleSet() > has a XXX about that): I've managed to create dead entries for a > role or a database in pg_db_role_setting while altering and dropping > a role/database in two concurrent sessions. Yeah, I was playing with that too. I think we need a few extra LockSharedObject calls, and not only in the new code :-( (This troubles me in the case of databases, because we already grab a lock on it during connection establishing, so this could cause extra contention there.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] TODO item: Allow more complex user/database default GUC settings
--On 30. September 2009 13:19:53 -0400 Alvaro Herrera wrote: I think it would be helpful if you could post ONE patch with all the changes and all the new files in the diff. AIUI, "the" patch is now split across three separate emails. :-( That's correct, here it is. Some additional notes: - ALTER ROLE ... IN DATABASE is missing some documentation. If you want, i can work on this. - The patch as is has still some locking problems (AlterRoleSet() has a XXX about that): I've managed to create dead entries for a role or a database in pg_db_role_setting while altering and dropping a role/database in two concurrent sessions. -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió:
> Alvaro Herrera writes:
> > FWIW I came up with a preliminary patch for a new psql command \dus that
> > shows settings. It takes a pattern that's used to constrain on roles.
> > Thus there is no way to view settings for a database. If there's a need
> > for that we could use another command, say \dls.
>
> Why not two pattern arguments?
>
> \drds [ role-pattern [ db-pattern ]]
Hmm, interesting idea, patch attached. This required changing the API
of processSQLNamePattern to return a bool indicating whether a clause
was added; otherwise, when processing the second pattern it was
impossible to figure out if we needed a WHERE or not.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
*** src/bin/pg_dump/dumputils.c 4 Aug 2009 21:56:08 - 1.48
--- src/bin/pg_dump/dumputils.c 30 Sep 2009 20:58:27 -
***
*** 894,900
*
* Scan a wildcard-pattern string and generate appropriate WHERE clauses
* to limit the set of objects returned. The WHERE clauses are appended
! * to the already-partially-constructed query in buf.
*
* conn: connection query will be sent to (consulted for escaping rules).
* buf: output parameter.
--- 894,901
*
* Scan a wildcard-pattern string and generate appropriate WHERE clauses
* to limit the set of objects returned. The WHERE clauses are appended
! * to the already-partially-constructed query in buf. Returns whether
! * any clause was added.
*
* conn: connection query will be sent to (consulted for escaping rules).
* buf: output parameter.
***
*** 913,919
* Formatting note: the text already present in buf should end with a newline.
* The appended text, if any, will end with one too.
*/
! void
processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
--- 914,920
* Formatting note: the text already present in buf should end with a newline.
* The appended text, if any, will end with one too.
*/
! bool
processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
***
*** 925,933
bool inquotes;
const char *cp;
int i;
#define WHEREAND() \
! (appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), have_where = true)
if (pattern == NULL)
{
--- 926,936
bool inquotes;
const char *cp;
int i;
+ bool added_clause = false;
#define WHEREAND() \
! (appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \
! have_where = true, added_clause = true)
if (pattern == NULL)
{
***
*** 937,943
WHEREAND();
appendPQExpBuffer(buf, "%s\n", visibilityrule);
}
! return;
}
initPQExpBuffer(&schemabuf);
--- 940,946
WHEREAND();
appendPQExpBuffer(buf, "%s\n", visibilityrule);
}
! return added_clause;
}
initPQExpBuffer(&schemabuf);
***
*** 1094,1098
--- 1097,1102
termPQExpBuffer(&schemabuf);
termPQExpBuffer(&namebuf);
+ return added_clause;
#undef WHEREAND
}
*** src/bin/pg_dump/dumputils.h 4 Aug 2009 21:56:08 - 1.25
--- src/bin/pg_dump/dumputils.h 30 Sep 2009 20:57:42 -
***
*** 36,42
const char *type, const char *acls, const char *owner,
int remoteVersion,
PQExpBuffer sql);
! extern void processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
--- 36,42
const char *type, const char *acls, const char *owner,
int remoteVersion,
PQExpBuffer sql);
! extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
*** src/bin/psql/command.c 13 Sep 2009 22:18:22 - 1.207
--- src/bin/psql/command.c 30 Sep 2009 20:47:53 -
***
*** 408,413
--- 408,426
case 's':
success = listTables(&cmd[1], pattern, show_verbose, show_system);
break;
+ case 'r':
+ if (cmd[2] == 'd' && cmd[3] == 's')
+ {
+ char *pattern2 = NULL;
+
+ if (pattern)
+ pattern2 = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, true);
+ success = listDbRoleSettings(pattern, pattern2);
+ }
+ else
+ success = PSQL_CMD_UNKNOWN;
+ break;
case 'u':
success = describeRoles(pattern, show_verbose);
break;
*** src/bin/psql/describe.c 29 Jul 2009 20:56:19 - 1.226
--- src/bin/psql/describe.c 30 Sep 2009 20:59:13 -
***
*** 2176,2181
--- 2176,2240
appendPQExpBufferStr(buf, str
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera writes: > FWIW I came up with a preliminary patch for a new psql command \dus that > shows settings. It takes a pattern that's used to constrain on roles. > Thus there is no way to view settings for a database. If there's a need > for that we could use another command, say \dls. Why not two pattern arguments? \drds [ role-pattern [ db-pattern ]] Omitted patterns are presumed to be * 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió:
> BTW, have we thought much about the simplest possible solution,
> which is to not have the view? How badly do we need it? Seems
> like dropping the functionality into a psql \d command might be
> a viable alternative.
FWIW I came up with a preliminary patch for a new psql command \dus that
shows settings. It takes a pattern that's used to constrain on roles.
Thus there is no way to view settings for a database. If there's a need
for that we could use another command, say \dls.
Sample output
alvherre=# \dus fo*
List of settings
role | database | settings
--+--+---
fob | | log_duration=true
foo | alvherre | work_mem=256MB
: statement_timeout=10s
foo | | work_mem=512MB
: statement_timeout=1s
(3 rows)
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
*** src/bin/psql/command.c 13 Sep 2009 22:18:22 - 1.207
--- src/bin/psql/command.c 30 Sep 2009 19:28:07 -
***
*** 409,415
success = listTables(&cmd[1], pattern, show_verbose, show_system);
break;
case 'u':
! success = describeRoles(pattern, show_verbose);
break;
case 'F': /* text search subsystem */
switch (cmd[2])
--- 409,418
success = listTables(&cmd[1], pattern, show_verbose, show_system);
break;
case 'u':
! if (cmd[2] && cmd[2] == 's')
! success = listRoleSettings(pattern);
! else
! success = describeRoles(pattern, show_verbose);
break;
case 'F': /* text search subsystem */
switch (cmd[2])
*** src/bin/psql/describe.c 29 Jul 2009 20:56:19 - 1.226
--- src/bin/psql/describe.c 30 Sep 2009 19:54:42 -
***
*** 2176,2181
--- 2176,2232
appendPQExpBufferStr(buf, str);
}
+ /*
+ * \dus
+ */
+ bool
+ listRoleSettings(const char *pattern)
+ {
+ PQExpBufferData buf;
+ PGresult *res;
+ printQueryOpt myopt = pset.popt;
+
+ initPQExpBuffer(&buf);
+
+ if (pset.sversion >= 80500)
+ {
+ printfPQExpBuffer(&buf, "SELECT rolname AS role, datname AS database,\n"
+ "pg_catalog.array_to_string(setconfig, E'\\n') AS settings\n"
+ "FROM pg_db_role_setting AS s\n"
+ "LEFT JOIN pg_database ON pg_database.oid = setdatabase\n"
+ "LEFT JOIN pg_roles ON pg_roles.oid = setrole ");
+ processSQLNamePattern(pset.db, &buf, pattern, false, false,
+ NULL, "pg_roles.rolname", NULL, NULL);
+ appendPQExpBufferStr(&buf, " ORDER BY role, database");
+ }
+ else
+ return false;
+
+ res = PSQLexec(buf.data, false);
+ if (!res)
+ return false;
+
+ if (PQntuples(res) == 0 && !pset.quiet)
+ {
+ if (pattern)
+ fprintf(pset.queryFout, _("No matching roles found.\n"));
+ else
+ fprintf(pset.queryFout, _("No settings found.\n"));
+ }
+ else
+ {
+ myopt.nullPrint = NULL;
+ myopt.title = _("List of settings");
+ myopt.translate_header = true;
+
+ printQuery(res, &myopt, pset.queryFout, pset.logfile);
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(&buf);
+ return true;
+ }
+
/*
* listTables()
*** src/bin/psql/describe.h 21 Apr 2009 15:49:06 - 1.40
--- src/bin/psql/describe.h 30 Sep 2009 19:29:20 -
***
*** 27,32
--- 27,35
/* \du, \dg */
extern bool describeRoles(const char *pattern, bool verbose);
+ /* \dus */
+ extern bool listRoleSettings(const char *pattern);
+
/* \z (or \dp) */
extern bool permissionsList(const char *pattern);
--
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] TODO item: Allow more complex user/database default GUC settings
On Wed, Sep 30, 2009 at 10:34 AM, Alvaro Herrera wrote: > And here's the last necessary bit, which is pg_dump support for all > this. I think it would be helpful if you could post ONE patch with all the changes and all the new files in the diff. AIUI, "the" patch is now split across three separate emails. :-( ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera writes: > Tom Lane escribió: >> Hmm ... I would kind of think that --roles-only should suppress this too. >> Otherwise you're going to be dumping commands that might refer to >> nonexistent databases. > Hmm. The problem I have with this idea is that the only way to dump the > per-database role settings is if you are also dumping the contents of > all databases. Which seems like a pain to me because the usage I > usually recommend is to backup global objects with pg_dumpall -g. Huh? --globals-only would still dump them, no? > I wonder if pg_dumpall should have a method for dumping database > creation and settings, excluding contents (leaving that for plain > pg_dump). Perhaps. People keep speculating about refactoring the division of labor between pg_dump and pg_dumpall. I'd advise leaving that for a separate patch though ... 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió:
> Alvaro Herrera writes:
> > And here's the last necessary bit, which is pg_dump support for all
> > this.
>
> > + /* Dump role/database settings */
> > + if (!tablespaces_only)
> > + {
> > + if (server_version >= 80500)
> > + dumpDbRoleConfig(conn);
> > + }
>
> Hmm ... I would kind of think that --roles-only should suppress this too.
> Otherwise you're going to be dumping commands that might refer to
> nonexistent databases.
Hmm. The problem I have with this idea is that the only way to dump the
per-database role settings is if you are also dumping the contents of
all databases. Which seems like a pain to me because the usage I
usually recommend is to backup global objects with pg_dumpall -g.
I wonder if pg_dumpall should have a method for dumping database
creation and settings, excluding contents (leaving that for plain
pg_dump).
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió:
> Alvaro Herrera writes:
> > And here's the last necessary bit, which is pg_dump support for all
> > this.
>
> > + /* Dump role/database settings */
> > + if (!tablespaces_only)
> > + {
> > + if (server_version >= 80500)
> > + dumpDbRoleConfig(conn);
> > + }
>
> Hmm ... I would kind of think that --roles-only should suppress this too.
> Otherwise you're going to be dumping commands that might refer to
> nonexistent databases.
Those double negatives are confusing as hell. I propose to add
something like this:
do_tablespaces = true;
do_databases = true;
do_roles = true;
if (globals_only)
do_databases = false;
if (tablespaces_only)
{
do_roles = false;
do_databases = false;
}
if (roles_only)
{
do_databases = false;
do_tablespaces = false;
}
Then we can have the new block this way:
/* Dump role/database settings */
if (do_databases && do_roles)
{
if (server_version >= 80500)
dumpDbRoleConfig(conn);
}
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera writes:
> And here's the last necessary bit, which is pg_dump support for all
> this.
> + /* Dump role/database settings */
> + if (!tablespaces_only)
> + {
> + if (server_version >= 80500)
> + dumpDbRoleConfig(conn);
> + }
Hmm ... I would kind of think that --roles-only should suppress this too.
Otherwise you're going to be dumping commands that might refer to
nonexistent databases.
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] TODO item: Allow more complex user/database default GUC settings
And here's the last necessary bit, which is pg_dump support for all
this.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
*** src/bin/pg_dump/pg_dumpall.c 11 Jun 2009 14:49:07 - 1.126
--- src/bin/pg_dump/pg_dumpall.c 30 Sep 2009 14:32:47 -
***
*** 43,50
static void dumpCreateDB(PGconn *conn);
static void dumpDatabaseConfig(PGconn *conn, const char *dbname);
static void dumpUserConfig(PGconn *conn, const char *username);
static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
! const char *type, const char *name);
static void dumpDatabases(PGconn *conn);
static void dumpTimestamp(char *msg);
static void doShellQuoting(PQExpBuffer buf, const char *str);
--- 43,52
static void dumpCreateDB(PGconn *conn);
static void dumpDatabaseConfig(PGconn *conn, const char *dbname);
static void dumpUserConfig(PGconn *conn, const char *username);
+ static void dumpDbRoleConfig(PGconn *conn);
static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
! const char *type, const char *name, const char *type2,
! const char *name2);
static void dumpDatabases(PGconn *conn);
static void dumpTimestamp(char *msg);
static void doShellQuoting(PQExpBuffer buf, const char *str);
***
*** 501,506
--- 503,515
/* Dump CREATE DATABASE commands */
if (!globals_only && !roles_only && !tablespaces_only)
dumpCreateDB(conn);
+
+ /* Dump role/database settings */
+ if (!tablespaces_only)
+ {
+ if (server_version >= 80500)
+ dumpDbRoleConfig(conn);
+ }
}
if (!globals_only && !roles_only && !tablespaces_only)
***
*** 1325,1339
{
PGresult *res;
! printfPQExpBuffer(buf, "SELECT datconfig[%d] FROM pg_database WHERE datname = ", count);
appendStringLiteralConn(buf, dbname, conn);
appendPQExpBuffer(buf, ";");
res = executeQuery(conn, buf->data);
! if (!PQgetisnull(res, 0, 0))
{
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "DATABASE", dbname);
PQclear(res);
count++;
}
--- 1334,1357
{
PGresult *res;
! if (server_version >= 80500)
! printfPQExpBuffer(buf, "SELECT setconfig[%d] FROM pg_db_role_setting WHERE "
! "setrole = 0 AND setdatabase = (SELECT oid FROM pg_database WHERE datname = ", count);
! else
! printfPQExpBuffer(buf, "SELECT datconfig[%d] FROM pg_database WHERE datname = ", count);
appendStringLiteralConn(buf, dbname, conn);
+
+ if (server_version >= 80500)
+ appendPQExpBuffer(buf, ")");
+
appendPQExpBuffer(buf, ";");
res = executeQuery(conn, buf->data);
! if (PQntuples(res) == 1 &&
! !PQgetisnull(res, 0, 0))
{
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "DATABASE", dbname, NULL, NULL);
PQclear(res);
count++;
}
***
*** 1362,1379
{
PGresult *res;
! if (server_version >= 80100)
printfPQExpBuffer(buf, "SELECT rolconfig[%d] FROM pg_authid WHERE rolname = ", count);
else
printfPQExpBuffer(buf, "SELECT useconfig[%d] FROM pg_shadow WHERE usename = ", count);
appendStringLiteralConn(buf, username, conn);
res = executeQuery(conn, buf->data);
if (PQntuples(res) == 1 &&
!PQgetisnull(res, 0, 0))
{
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "ROLE", username);
PQclear(res);
count++;
}
--- 1380,1403
{
PGresult *res;
! if (server_version >= 80500)
! printfPQExpBuffer(buf, "SELECT setconfig[%d] FROM pg_db_role_setting WHERE "
! "setdatabase = 0 AND setrole = "
! "(SELECT oid FROM pg_authid WHERE rolname = ", count);
! else if (server_version >= 80100)
printfPQExpBuffer(buf, "SELECT rolconfig[%d] FROM pg_authid WHERE rolname = ", count);
else
printfPQExpBuffer(buf, "SELECT useconfig[%d] FROM pg_shadow WHERE usename = ", count);
appendStringLiteralConn(buf, username, conn);
+ if (server_version >= 80500)
+ appendPQExpBuffer(buf, ")");
res = executeQuery(conn, buf->data);
if (PQntuples(res) == 1 &&
!PQgetisnull(res, 0, 0))
{
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "ROLE", username, NULL, NULL);
PQclear(res);
count++;
}
***
*** 1388,1400
}
/*
* Helper function for dumpXXXConfig().
*/
static void
makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
! const char *type, const char *name)
{
char *pos;
char *mine;
--- 1412,1458
}
+ /*
+ * Dump user-and-database-specific configuration
+ */
+ static void
+ dumpDbRoleConfig(PGconn *conn)
+ {
+ PQExpBuffer buf = createPQExpBuffer();
+ PGresult *res;
+ int i;
+
+ printfPQExpBuffer(buf, "SELECT rolname, datname, unnest(setconfig) "
+ "F
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
--On 28. September 2009 19:02:34 -0400 Alvaro Herrera wrote: Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't compile anymore with this error: Here they are. I'll see if i can get to it tonight. I'm currently travelling, so it could be delayed until Thursday. -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Bernd Helmle escribió:
> Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't
> compile anymore with this error:
Here they are.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
/*
* pg_db_role_setting.c
* Routines to support manipulation of the pg_db_role_setting relation
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL$
*/
#include "postgres.h"
#include "access/genam.h"
#include "access/heapam.h"
#include "access/htup.h"
#include "access/skey.h"
#include "catalog/indexing.h"
#include "catalog/pg_db_role_setting.h"
#include "utils/fmgroids.h"
#include "utils/rel.h"
#include "utils/tqual.h"
void
AlterSetting(Oid databaseid, Oid roleid, VariableSetStmt *setstmt)
{
char *valuestr;
HeapTuple tuple;
Relation rel;
ScanKeyData scankey[2];
SysScanDesc scan;
valuestr = ExtractSetVariableArgs(setstmt);
/* Get the old tuple, if any. */
rel = heap_open(DbRoleSettingRelationId, RowExclusiveLock);
ScanKeyInit(&scankey[0],
Anum_pg_db_role_setting_setdatabase,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(databaseid));
ScanKeyInit(&scankey[1],
Anum_pg_db_role_setting_setrole,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(roleid));
scan = systable_beginscan(rel, DbRoleSettingDatidRolidIndexId, true,
SnapshotNow, 2, scankey);
tuple = systable_getnext(scan);
/*
* There are three cases:
*
* - in RESET ALL, simply delete the pg_db_role_setting tuple (if any)
*
* - in other commands, if there's a tuple in pg_db_role_setting, update it;
* if it ends up empty, delete it
*
* - otherwise, insert a new pg_db_role_setting tuple, but only if the
* command is not RESET
*/
if (setstmt->kind == VAR_RESET_ALL)
{
if (HeapTupleIsValid(tuple))
simple_heap_delete(rel, &tuple->t_self);
}
else if (HeapTupleIsValid(tuple))
{
Datum repl_val[Natts_pg_db_role_setting];
bool repl_null[Natts_pg_db_role_setting];
bool repl_repl[Natts_pg_db_role_setting];
HeapTuple newtuple;
Datum datum;
bool isnull;
ArrayType *a;
memset(repl_repl, false, sizeof(repl_repl));
repl_repl[Anum_pg_db_role_setting_setconfig - 1] = true;
repl_null[Anum_pg_db_role_setting_setconfig - 1] = false;
/* Extract old value of setconfig */
datum = heap_getattr(tuple, Anum_pg_db_role_setting_setconfig,
RelationGetDescr(rel), &isnull);
a = isnull ? NULL : DatumGetArrayTypeP(datum);
/* Update (valuestr is NULL in RESET cases) */
if (valuestr)
a = GUCArrayAdd(a, setstmt->name, valuestr);
else
a = GUCArrayDelete(a, setstmt->name);
if (a)
{
repl_val[Anum_pg_db_role_setting_setconfig - 1] =
PointerGetDatum(a);
newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel),
repl_val, repl_null, repl_repl);
simple_heap_update(rel, &tuple->t_self, newtuple);
/* Update indexes */
CatalogUpdateIndexes(rel, newtuple);
}
else
simple_heap_delete(rel, &tuple->t_self);
}
else if (valuestr)
{
/* non-null valuestr means it's not RESET, so insert a new tuple */
HeapTuple newtuple;
Datum values[Natts_pg_db_role_setting];
bool nulls[Natts_pg_db_role_setting];
ArrayType *a;
memset(nulls, false, sizeof(nulls));
a = GUCArrayAdd(NULL, setstmt->name, valuestr);
values[Anum_pg_db_role_setting_setdatabase - 1] =
ObjectIdGetDatum(databaseid);
values[Anum_pg_db_role_setting_setrole - 1] = ObjectIdGetDatum(roleid);
values[Anum_pg_db_role_setting_setconfig - 1] = PointerGetDatum(a);
newtuple = heap_form_tuple(RelationGetDescr(rel), values, nulls);
simple_heap_insert(rel, newtuple);
/* Update indexes */
CatalogUpdateIndexes(rel, newtuple);
}
systable_endscan(scan);
/* Close pg_db_role_setting, but keep lock till commit */
heap_close(rel, NoLock);
}
/*
* Drop some settings from the catalog. These can be for a particular
* database, or for a particular role. (It is of course possible to do both
* too, but it doesn't make sense for current uses.)
*/
void
DropSetting(Oid databaseid, Oid roleid)
{
Relation relsetting;
HeapScanDesc scan;
ScanKeyData keys[2];
HeapTuple tup;
intnumkeys = 0;
relsetting = heap_open(DbRoleSettingRelationId, RowExclusiveLock);
if (OidIsValid(databaseid))
{
ScanKeyInit(&keys[numkeys],
Anum_pg_db_role_setting_setdatabase,
BTEqualStrategyNumber,
F_OIDEQ,
ObjectIdGetDatum(databaseid));
numkeys++;
}
if (OidIsValid(roleid))
{
ScanKeyInit(&keys[numkeys],
Anum_pg_db_role_setting_setrole,
BTEqualStrategyNumber,
F_OIDEQ,
ObjectIdGetDatum(roleid));
numkeys++;
}
scan = heap_beginscan(relsetting, SnapshotNow, numkeys, keys);
while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
{
simple_heap_delete(relsetting, &tup->t_self);
}
heap_endscan(sca
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
Bernd Helmle escribió: > Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't > compile anymore with this error: Huh, you're right, I did :-( Let me unpack the laptop ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] TODO item: Allow more complex user/database default GUC settings
On Sep 27, 2009, at 9:19 PM, Tom Lane wrote: What we seem to be lacking in this case is a good tech-speak option for the underlying catalog name. I'm still not happy with having a catalog and a view that are exactly the same except for "s", especially since as Alvaro notes that won't lead to desirable tab-completion behavior. OTOH, we have survived with pg_index vs pg_indexes, so maybe it wouldn't kill us. Another option is to revisit the set of system views (http:// pgfoundry.org/projects/newsysviews/). IIRC there was some other recent reason we wanted to do that. -- Decibel!, aka Jim C. Nasby, Database Architect deci...@decibel.org Give your computer some brain candy! www.distributed.net Team #1828 -- 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] TODO item: Allow more complex user/database default GUC settings
--On 27. September 2009 21:59:37 -0400 Robert Haas wrote: Bernd, Can you review this new version and mark this as Ready for Committer if it looks OK, or else respond with comments and set it back to Waiting on Author? Seems Alvaro forgot to include pg_db_role_setting.h, it doesn't compile anymore with this error: catalog.c:34:40: error: catalog/pg_db_role_setting.h: No such file or directory catalog.c: In function ‘IsSharedRelation’: catalog.c:311: error: ‘DbRoleSettingRelationId’ undeclared (first use in this function) -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Robert Haas writes: > That seems to me to be just confusing the issue. Now the table name > and the view name are just totally different with no obvious > connection between them. We have enough nonsense of this type already > (e.g. pg_stats vs. pg_statistic; pg_authid vs. pg_roles vs. > pg_shadow). I think we need to settle on a system for handling > problems of this type and document it in the fine manual or perhaps a > README somewhere, and stick with it. Inventing random unconnected > names is just craziness. Actually, to the extent that we have any convention at all, it's to use plurals for system view names (pg_tables, pg_views, etc) and singular for underlying catalogs (pg_index). The only exception to this on the catalog side is pg_auth_members, which is arguably misnamed. (pg_inherits is sort of an exception, but it's weird in a different way: its name is a verb not a noun.) The apparent exceptions on the view side (pg_group, pg_shadow, pg_user) are actually views that are backward compatible substitutes for former catalogs, so they are not really intentional exceptions. Now it's also the case that we've tended to use tech-speak names for catalogs (eg, pg_class, pg_namespace not pg_table, pg_schema) and so that gives us an additional degree of separation between those names and the more user-facing names chosen for views. What we seem to be lacking in this case is a good tech-speak option for the underlying catalog name. I'm still not happy with having a catalog and a view that are exactly the same except for "s", especially since as Alvaro notes that won't lead to desirable tab-completion behavior. OTOH, we have survived with pg_index vs pg_indexes, so maybe it wouldn't kill us. BTW, have we thought much about the simplest possible solution, which is to not have the view? How badly do we need it? Seems like dropping the functionality into a psql \d command might be a viable alternative. 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] TODO item: Allow more complex user/database default GUC settings
On Fri, Sep 25, 2009 at 8:05 PM, Alvaro Herrera wrote: > Alvaro Herrera escribió: > >> I think it can be solved by splitting OptRoleElem in a set of >> productions for ALTER and a superset of that for ALTER. I'll go try >> that. > > Right, that works. Updated patch attached; should solve the issues > raised in the thread. I renamed the catalog pg_db_role_setting as > suggested by Tom. > > I have updated the pg_user and pg_roles definitions so that they include > the settings for the role, but only those that are not specific to any > database. > > I have also added a view, whose only purpose is to convert the role and > database OIDs into names. It's been named pg_db_role_settings, but if > anyone has a better suggestion I'm all ears. Bernd, Can you review this new version and mark this as Ready for Committer if it looks OK, or else respond with comments and set it back to Waiting on Author? Thanks, ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
On Sat, Sep 26, 2009 at 11:44 PM, Alvaro Herrera wrote: > Robert Haas escribió: > >> The problem of having both a table and a closely related view is, >> IME, one that comes up a lot. I think you just need to pick a >> convention and stick with it. Mine is to append "_view" to the >> table name. > > That would make the difference clear, but since what the user normally > wants to see is the view, it seems a poor solution to make the view the > more difficult one to type (and the one that isn't tab-completed first > in psql). I'd go with naming the view pg_db_role_setting and append > "_internal" to the catalog or something similar, except that we don't > have any catalog with such a bad name yet and I don't want to start. > > Maybe name the table pg_configuration? That seems to me to be just confusing the issue. Now the table name and the view name are just totally different with no obvious connection between them. We have enough nonsense of this type already (e.g. pg_stats vs. pg_statistic; pg_authid vs. pg_roles vs. pg_shadow). I think we need to settle on a system for handling problems of this type and document it in the fine manual or perhaps a README somewhere, and stick with it. Inventing random unconnected names is just craziness. Now, if you/others don't like my _view convention; that's fine. Just pick something else. Really, I don't believe the tab-completion thing is much of a problem, you just type underscore-tab and you're there. But I am 100% OK with whatever we pick, as long as it is something easy to remember that we have a chance of being able to apply consistently. ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
Robert Haas escribió: > The problem of having both a table and a closely related view is, > IME, one that comes up a lot. I think you just need to pick a > convention and stick with it. Mine is to append "_view" to the > table name. That would make the difference clear, but since what the user normally wants to see is the view, it seems a poor solution to make the view the more difficult one to type (and the one that isn't tab-completed first in psql). I'd go with naming the view pg_db_role_setting and append "_internal" to the catalog or something similar, except that we don't have any catalog with such a bad name yet and I don't want to start. Maybe name the table pg_configuration? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] TODO item: Allow more complex user/database default GUC settings
On Sep 26, 2009, at 11:59 AM, Tom Lane wrote: Alvaro Herrera writes: Right, that works. Updated patch attached; should solve the issues raised in the thread. I renamed the catalog pg_db_role_setting as suggested by Tom. ... I have also added a view, whose only purpose is to convert the role and database OIDs into names. It's been named pg_db_role_settings, but if anyone has a better suggestion I'm all ears. I dislike the idea of having a catalog and a view whose names are the same except for a plural. It's confusing as heck, because no one will remember which is which. Since pg_settings is the existing user view, I think pg_db_role_settings is a reasonable choice for the new view, but then we need a different name for the catalog. The only thing that comes to mind right now is "pg_db_role_default", but I don't like it much. Anybody have other suggestions? The problem of having both a table and a closely related view is, IME, one that comes up a lot. I think you just need to pick a convention and stick with it. Mine is to append "_view" to the table name. Renaming the underlying table doesn't seem like it helps at all. ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera writes: > Right, that works. Updated patch attached; should solve the issues > raised in the thread. I renamed the catalog pg_db_role_setting as > suggested by Tom. > ... > I have also added a view, whose only purpose is to convert the role and > database OIDs into names. It's been named pg_db_role_settings, but if > anyone has a better suggestion I'm all ears. I dislike the idea of having a catalog and a view whose names are the same except for a plural. It's confusing as heck, because no one will remember which is which. Since pg_settings is the existing user view, I think pg_db_role_settings is a reasonable choice for the new view, but then we need a different name for the catalog. The only thing that comes to mind right now is "pg_db_role_default", but I don't like it much. Anybody have other suggestions? 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] TODO item: Allow more complex user/database default GUC settings
Tom Lane escribió: > Gurjeet Singh writes: > > ON instead of second ALTER looks better, and IMHO DATABASE should > > be optional too: > > > ALTER ROLE [ON DATABASE ] SET TO ; > > IN, not ON. This creates a parser conflict with CREATE ROLE foo IN ROLE bar I think it can be solved by splitting OptRoleElem in a set of productions for ALTER and a superset of that for ALTER. I'll go try that. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] TODO item: Allow more complex user/database default GUC settings
On Wed, Sep 23, 2009 at 3:03 PM, Bernd Helmle wrote: > > > --On 23. September 2009 14:10:39 -0400 Tom Lane wrote: > >> FWIW, I looked the patch over quickly, and I think it will be fine once >> Bernd's comments are addressed. In particular I agree with the >> objection to the name "pg_setting" as being confusingly close to >> "pg_settings". But "pg_user_setting" isn't better. Maybe >> "pg_db_role_settings"? > > Jepp, that's better, +1 from me. > > I'm done with this, too, so i will mark this as "Returned with Feedback", if > no one objects? It can be marked "Waiting on Author" if it's going to be reworked in the next few days. If no plans to rework, or if the rework doesn't materialize, then "Returned with Feedback". ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
--On 23. September 2009 14:10:39 -0400 Tom Lane wrote: FWIW, I looked the patch over quickly, and I think it will be fine once Bernd's comments are addressed. In particular I agree with the objection to the name "pg_setting" as being confusingly close to "pg_settings". But "pg_user_setting" isn't better. Maybe "pg_db_role_settings"? Jepp, that's better, +1 from me. I'm done with this, too, so i will mark this as "Returned with Feedback", if no one objects? -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera writes: > Robert Haas escribió: >> So here's the followup question - do you intend to do one of those >> things for this CommitFest, or should we mark this as Returned with >> Feedback once Bernd posts the rest of his review? > What feedback is it supposed to be returned with? Precisely what I > wanted is some feedback on the general idea. Brendan's "I like the > approach" is good, but is it enough to deter a later veto from someone > else? FWIW, I looked the patch over quickly, and I think it will be fine once Bernd's comments are addressed. In particular I agree with the objection to the name "pg_setting" as being confusingly close to "pg_settings". But "pg_user_setting" isn't better. Maybe "pg_db_role_settings"? As far as the lock issue goes, I don't see any reason why the catalog change creates a reason for new/different locking than we had before. Any attempt to make concurrent updates to the same row will generate an error, and that seems enough to me ... 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] TODO item: Allow more complex user/database default GUC settings
Gurjeet Singh writes: > ON instead of second ALTER looks better, and IMHO DATABASE should > be optional too: > ALTER ROLE [ON DATABASE ] SET TO ; IN, not ON. 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] TODO item: Allow more complex user/database default GUC settings
On Tue, Sep 22, 2009 at 4:16 AM, Bernd Helmle wrote:
>
>
> --On 21. September 2009 13:42:21 +0200 Bernd Helmle
> wrote:
>
>
>>
>> --On 20. September 2009 22:56:53 -0400 Robert Haas
>> wrote:
>>
>> So is this ready to commit, or what?
>>>
>>
>> Not yet, see the comments Alvaro did upthread. Please note that i'm still
>> reviewing this one and i hope to post results tomorrow (there wasn't
>> plenty of free time over the weekend, i'm sorry).
>>
>>
> Here some further comments on the current patch:
>
> - I'm not sure i like the name of the new system catalog pg_setting. Wie
> already have pg_settings, i think this can be confusing. Maybe we need a
> different name, e.g. pg_user_setting? This seems along the line with the
> other *user* system objects (e.g. pg_stat_user_tables), where only "user
> specific" objects are displayed.
>
> - I have thought a little bit about the changes in the system views.
> pg_roles and pg_shadow (as Alvaro already mentioned), need to be adjusted
> (joined to the new catalog), to display rolconfig/useconfig. However, it's
> unclear *how* to expose those information, for example, do we want to expose
> roleconfig specific for the current database or for all databases the role
> has a specific config for ?
>
> - The code mentions the lack of lock synchronization. Maybe i'm missing
> something obvious (its late here), but is there a reason this can't be done
> by obtaining a lock on pg_authid (not sure about the backend user
> initialization phase though) ?
>
> - Regarding the missing UI: i go with Alvaro's proposal:
>
> ALTER ROLE [ALTER] DATABASE SET TO ;
>
> Maybe we can make the 2nd ALTER optional.
>
> Thoughts?
ON instead of second ALTER looks better, and IMHO DATABASE should
be optional too:
ALTER ROLE [ON DATABASE ] SET TO ;
Best regards,
--
Lets call it Postgres
EnterpriseDB http://www.enterprisedb.com
gurjeet[.sin...@enterprisedb.com
singh.gurj...@{ gmail | hotmail | indiatimes | yahoo }.com
Twitter: singh_gurjeet
Skype: singh_gurjeet
Mail sent from my BlackLaptop device
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
On Mon, Sep 21, 2009 at 12:23 PM, Alvaro Herrera wrote: > Robert Haas escribió: > >> So here's the followup question - do you intend to do one of those >> things for this CommitFest, or should we mark this as Returned with >> Feedback once Bernd posts the rest of his review? > > What feedback is it supposed to be returned with? Precisely what I > wanted is some feedback on the general idea. Brendan's "I like the > approach" is good, but is it enough to deter a later veto from someone > else? Well, you've hit there on one of the things that we don't always do well. Many a patch author has posted an idea, received no feedback, proceeded to implementation, and then the knives come out. On a good day, the CommitFest process ensures that every patch gets a second opinion, but it doesn't guarantee that a third opinion won't come crawling out of the woodwork at a later date. In this respect, you're actually operating at a slight advantage relative to most of us, because you can post your revised patch and commit it if no one objects too strongly, whereas I (for example) have to convince one of about two people - Tom or Peter, for nearly anything I'm likely to develop - to take an affirmative action on my behalf. This whole phenomenon of proposals to which no objection was made at the outset later getting flak for one reason or another is, I think, a source of much frustration and discourages people from putting effort into projects they might otherwise be willing to undertake. But I haven't the least idea how to fix it, and I can't offer you any guarantees with respect to the present situation either. ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
--On 21. September 2009 13:42:21 +0200 Bernd Helmle wrote: --On 20. September 2009 22:56:53 -0400 Robert Haas wrote: So is this ready to commit, or what? Not yet, see the comments Alvaro did upthread. Please note that i'm still reviewing this one and i hope to post results tomorrow (there wasn't plenty of free time over the weekend, i'm sorry). Here some further comments on the current patch: - I'm not sure i like the name of the new system catalog pg_setting. Wie already have pg_settings, i think this can be confusing. Maybe we need a different name, e.g. pg_user_setting? This seems along the line with the other *user* system objects (e.g. pg_stat_user_tables), where only "user specific" objects are displayed. - I have thought a little bit about the changes in the system views. pg_roles and pg_shadow (as Alvaro already mentioned), need to be adjusted (joined to the new catalog), to display rolconfig/useconfig. However, it's unclear *how* to expose those information, for example, do we want to expose roleconfig specific for the current database or for all databases the role has a specific config for ? - The code mentions the lack of lock synchronization. Maybe i'm missing something obvious (its late here), but is there a reason this can't be done by obtaining a lock on pg_authid (not sure about the backend user initialization phase though) ? - Regarding the missing UI: i go with Alvaro's proposal: ALTER ROLE [ALTER] DATABASE SET TO ; Maybe we can make the 2nd ALTER optional. Thoughts? -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera escribió: > What feedback is it supposed to be returned with? Precisely what I > wanted is some feedback on the general idea. Brendan's "I like the > approach" is good, but is it enough to deter a later veto from someone > else? s/Brendan/Bernd/ -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] TODO item: Allow more complex user/database default GUC settings
Robert Haas escribió: > So here's the followup question - do you intend to do one of those > things for this CommitFest, or should we mark this as Returned with > Feedback once Bernd posts the rest of his review? What feedback is it supposed to be returned with? Precisely what I wanted is some feedback on the general idea. Brendan's "I like the approach" is good, but is it enough to deter a later veto from someone else? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] TODO item: Allow more complex user/database default GUC settings
On Mon, Sep 21, 2009 at 12:21 AM, Alvaro Herrera wrote: > Robert Haas escribió: > >> > Here's a first shot on this for my current review round. Patch needed to >> > re-merged into current CVS HEAD due to some merge conflicts, i've also >> > adjusted the regression tests (minor). On a first look, i like the patch >> > (especially the code for the utility commands accessing the settings is >> > better modularized now, which looks much nicer). >> >> So is this ready to commit, or what? > > Not really :-( It needs some minor tweaks to qualify as a cleanup > patch, and further extra coding for there to be an actual new feature. So here's the followup question - do you intend to do one of those things for this CommitFest, or should we mark this as Returned with Feedback once Bernd posts the rest of his review? ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
--On 20. September 2009 22:56:53 -0400 Robert Haas wrote: So is this ready to commit, or what? Not yet, see the comments Alvaro did upthread. Please note that i'm still reviewing this one and i hope to post results tomorrow (there wasn't plenty of free time over the weekend, i'm sorry). -- Thanks Bernd -- 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] TODO item: Allow more complex user/database default GUC settings
Robert Haas escribió: > > Here's a first shot on this for my current review round. Patch needed to > > re-merged into current CVS HEAD due to some merge conflicts, i've also > > adjusted the regression tests (minor). On a first look, i like the patch > > (especially the code for the utility commands accessing the settings is > > better modularized now, which looks much nicer). > > So is this ready to commit, or what? Not really :-( It needs some minor tweaks to qualify as a cleanup patch, and further extra coding for there to be an actual new feature. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] TODO item: Allow more complex user/database default GUC settings
On Fri, Sep 18, 2009 at 4:03 PM, Bernd Helmle wrote: > > > --On 25. August 2009 22:17:38 -0400 Alvaro Herrera > wrote: > >> I'm just posting in case somebody has thoughts on the UI part of it. >> >> Other things that need fixed: >> >> - need to figure out locking for roles; this stuff must be synchronized >> with role drop >> - pg_shadow and pg_roles need a join to obtain settings >> - two regression tests need their expected file updated >> - catalog version bump > > Here's a first shot on this for my current review round. Patch needed to > re-merged into current CVS HEAD due to some merge conflicts, i've also > adjusted the regression tests (minor). On a first look, i like the patch > (especially the code for the utility commands accessing the settings is > better modularized now, which looks much nicer). So is this ready to commit, or what? ...Robert -- 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] TODO item: Allow more complex user/database default GUC settings
--On 25. August 2009 22:17:38 -0400 Alvaro Herrera wrote: I'm just posting in case somebody has thoughts on the UI part of it. Other things that need fixed: - need to figure out locking for roles; this stuff must be synchronized with role drop - pg_shadow and pg_roles need a join to obtain settings - two regression tests need their expected file updated - catalog version bump Here's a first shot on this for my current review round. Patch needed to re-merged into current CVS HEAD due to some merge conflicts, i've also adjusted the regression tests (minor). On a first look, i like the patch (especially the code for the utility commands accessing the settings is better modularized now, which looks much nicer). -- Thanks Bernd complex_guc_review_v1.patch Description: Binary data -- 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera wrote: > Implementation-side, it requires a new catalog (pg_settings), with the > following columns: So, I've come up with the attached patch. It does not have the new command yet, so you can do ALTER USER and ALTER DATABASE and it works, but there's no way to set user-and-database-specific settings, short of INSERT into the catalog. Apart from that it works nicely. I'm just posting in case somebody has thoughts on the UI part of it. Other things that need fixed: - need to figure out locking for roles; this stuff must be synchronized with role drop - pg_shadow and pg_roles need a join to obtain settings - two regression tests need their expected file updated - catalog version bump -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Index: doc/src/sgml/catalogs.sgml === RCS file: /home/alvherre/Code/cvs/pgsql/doc/src/sgml/catalogs.sgml,v retrieving revision 2.206 diff -c -p -r2.206 catalogs.sgml *** doc/src/sgml/catalogs.sgml 10 Aug 2009 22:13:50 - 2.206 --- doc/src/sgml/catalogs.sgml 26 Aug 2009 02:01:11 - *** *** 199,204 --- 199,209 + pg_setting + per-user and per-database settings + + + pg_shdepend dependencies on shared objects *** *** 2132,2144 - datconfig - text[] - - Session defaults for run-time configuration variables - - - datacl aclitem[] --- 2137,2142 *** *** 3996,4001 --- 3994,4058 + + pg_setting + + +pg_setting + + + +The catalog pg_setting records the default +values that have been set for run-time configuration variables, +for each role and database combination. + + + +Unlike most system catalogs, pg_setting +is shared across all databases of a cluster: there is only one +copy of pg_setting per cluster, not +one per database. + + + +pg_setting Columns + + + + + Name + Type + References + Description + + + + + + setdatabase + oid + pg_database.oid + The OID of the database the setting is applicable to, or zero if not database-specific + + + + setrole + oid + pg_authid.oid + The OID of the role the setting is applicable to, or zero if not role-specific + + + + setconfig + text[] + + Defaults for run-time configuration variables + + + + + + pg_shdepend *** *** 6448,6460 - rolconfig - text[] - - Session defaults for run-time configuration variables - - - oid oid pg_authid.oid --- 6505,6510 Index: src/backend/catalog/Makefile === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/catalog/Makefile,v retrieving revision 1.70 diff -c -p -r1.70 Makefile *** src/backend/catalog/Makefile 12 May 2009 00:56:05 - 1.70 --- src/backend/catalog/Makefile 25 Aug 2009 19:15:03 - *** include $(top_builddir)/src/Makefile.glo *** 13,19 OBJS = catalog.o dependency.o heap.o index.o indexing.o namespace.o aclchk.o \ pg_aggregate.o pg_constraint.o pg_conversion.o pg_depend.o pg_enum.o \ pg_inherits.o pg_largeobject.o pg_namespace.o pg_operator.o pg_proc.o \ !pg_shdepend.o pg_type.o storage.o toasting.o BKIFILES = postgres.bki postgres.description postgres.shdescription --- 13,19 OBJS = catalog.o dependency.o heap.o index.o indexing.o namespace.o aclchk.o \ pg_aggregate.o pg_constraint.o pg_conversion.o pg_depend.o pg_enum.o \ pg_inherits.o pg_largeobject.o pg_namespace.o pg_operator.o pg_proc.o \ !pg_setting.o pg_shdepend.o pg_type.o storage.o toasting.o BKIFILES = postgres.bki postgres.description postgres.shdescription *** POSTGRES_BKI_SRCS = $(addprefix $(top_sr *** 32,38 pg_language.h pg_largeobject.h pg_aggregate.h pg_statistic.h \ pg_rewrite.h pg_trigger.h pg_listener.h pg_description.h pg_cast.h \ pg_enum.h pg_namespace.h pg_conversion.h pg_depend.h \ ! pg_database.h pg_tablespace.h pg_pltemplate.h \ pg_authid.h pg_auth_members.h pg_shdepend.h pg_shdescription.h \ pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \ pg_ts_parser.h pg_ts_template.h \ --- 32,38 pg_language.h pg_largeobject.h pg_aggregate.h pg_statistic.h \ pg_rewrite.h pg_trigger.h pg_listener.h pg_description.h pg_cast.h \ pg_enum.h pg_namespace.h pg_conversion.h pg_depend.h \ ! pg_database.h pg_setting.h pg_tablespace.h pg_pltemplate.h \ pg_authid.h pg_auth_members
