Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 7:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Phil Sorber p...@omniti.com writes: Then there is a separate section of code that is called as a separate function 'dumpUserConfig()' that does other role attributes like 'ALTER ROLE bob SET role TO charlie'. These are the ALTER's that can have dependencies on other roles. Right. Phrased that way, this is an obvious improvement, and I concur that it doesn't look like it could break anything that works now. The more general problem remains to be solved, but we might as well apply this. OK, done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 02:27:51PM -0400, Phil Sorber wrote: My proposal would be to table the discussion about the 'ALTER DATABASE SET ROLE' case because there seems to be a bit of a philosophical debate behind this that needs to be sorted out first. Note: to table the discussion means opposite things in American and British english. I think the rest of the sentence makes it clear what you mean though :). Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Mon, Oct 10, 2011 at 11:54 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Aug 4, 2011 at 2:04 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Aug 4, 2011 at 1:53 PM, Phil Sorber p...@omniti.com wrote: Ok, here is the patch that just moves the ALTER/SET pieces to the end. Can we get this included in the next commit fest? Yep, just make yourself an account and add it. Unfortunately, it doesn't look like anyone ever replied to this thread, but Tom posted some thoughts on another thread that seem to me to be a serious problem for this patch: http://archives.postgresql.org/message-id/13764.1315094...@sss.pgh.pa.us I don't see any easy way around that problem, so I'm going to mark this patch Returned with Feedback for now. It strikes me as craziness to try to guess which settings we should restore at the beginning and which at the end, so I think we need a better idea. I don't really understand why it's not OK to just have pg_dump issue RESET ROLE at appropriate points in the process; that seems like it would be sufficient and not particularly ugly. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company I am going to remove that patch from the commit fest because we all agree that it does not solve the problem satisfactorily. I would like to re-iterate a few points, and submit a new patch. First off, there are two distinct problems here that we have been lumping into one. There is the issue of the 'ALTER DATABASE SET ROLE' and then there is the 'ALTER ROLE SET ROLE' case. The former is the one that has been causing us so many problems, and the latter is the one that I really care about. Also, there are more people that are hitting this issue as well: http://archives.postgresql.org/pgsql-hackers/2011-02/msg02362.php My proposal would be to table the discussion about the 'ALTER DATABASE SET ROLE' case because there seems to be a bit of a philosophical debate behind this that needs to be sorted out first. If we focus only on the 'ALTER ROLE' case I think there is a trivial solution. We already separate the CREATE from the ALTER in a single loop. We also already separate out the user config in a separate function called from this same loop. The problem is that the user config can be dependent upon a later CREATE. So all I suggest is to move the user config dumping into a new loop afterward so that the user config ALTER's come after all the other CREATE's and ALTER's. It amounts to a 7 line change and solves our problem rather elegantly. diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c new file mode 100644 index b5f64e8..ee597d5 *** a/src/bin/pg_dump/pg_dumpall.c --- b/src/bin/pg_dump/pg_dumpall.c *** dumpRoles(PGconn *conn) *** 804,814 buf, ROLE, rolename); fprintf(OPF, %s, buf-data); - - if (server_version = 70300) - dumpUserConfig(conn, rolename); } PQclear(res); fprintf(OPF, \n\n); --- 804,815 buf, ROLE, rolename); fprintf(OPF, %s, buf-data); } + if (server_version = 70300) + for (i = 0; i PQntuples(res); i++) + dumpUserConfig(conn, PQgetvalue(res, i, i_rolname)); + PQclear(res); fprintf(OPF, \n\n); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 2:27 PM, Phil Sorber p...@omniti.com wrote: I am going to remove that patch from the commit fest because we all agree that it does not solve the problem satisfactorily. I would like to re-iterate a few points, and submit a new patch. First off, there are two distinct problems here that we have been lumping into one. There is the issue of the 'ALTER DATABASE SET ROLE' and then there is the 'ALTER ROLE SET ROLE' case. The former is the one that has been causing us so many problems, and the latter is the one that I really care about. Also, there are more people that are hitting this issue as well: http://archives.postgresql.org/pgsql-hackers/2011-02/msg02362.php My proposal would be to table the discussion about the 'ALTER DATABASE SET ROLE' case because there seems to be a bit of a philosophical debate behind this that needs to be sorted out first. If we focus only on the 'ALTER ROLE' case I think there is a trivial solution. We already separate the CREATE from the ALTER in a single loop. We also already separate out the user config in a separate function called from this same loop. The problem is that the user config can be dependent upon a later CREATE. So all I suggest is to move the user config dumping into a new loop afterward so that the user config ALTER's come after all the other CREATE's and ALTER's. It amounts to a 7 line change and solves our problem rather elegantly. I'm not sure I completely follow this explanation of the problem, but it does seem better to me to set all the role attributes after dumping all the create role statements, and I don't see how that can break anything that works now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 3:48 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Oct 12, 2011 at 2:27 PM, Phil Sorber p...@omniti.com wrote: I am going to remove that patch from the commit fest because we all agree that it does not solve the problem satisfactorily. I would like to re-iterate a few points, and submit a new patch. First off, there are two distinct problems here that we have been lumping into one. There is the issue of the 'ALTER DATABASE SET ROLE' and then there is the 'ALTER ROLE SET ROLE' case. The former is the one that has been causing us so many problems, and the latter is the one that I really care about. Also, there are more people that are hitting this issue as well: http://archives.postgresql.org/pgsql-hackers/2011-02/msg02362.php My proposal would be to table the discussion about the 'ALTER DATABASE SET ROLE' case because there seems to be a bit of a philosophical debate behind this that needs to be sorted out first. If we focus only on the 'ALTER ROLE' case I think there is a trivial solution. We already separate the CREATE from the ALTER in a single loop. We also already separate out the user config in a separate function called from this same loop. The problem is that the user config can be dependent upon a later CREATE. So all I suggest is to move the user config dumping into a new loop afterward so that the user config ALTER's come after all the other CREATE's and ALTER's. It amounts to a 7 line change and solves our problem rather elegantly. I'm not sure I completely follow this explanation of the problem, but it does seem better to me to set all the role attributes after dumping all the create role statements, and I don't see how that can break anything that works now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Just to be clear, this doesn't move all the ALTER's. It will still do CREATE/ALTER pair's for attributes that could go in the CREATE. Example: CREATE ROLE bob; ALTER ROLE bob WITH LOGIN PASSWORD 'blah'; You could turn those in to one statement, but for various reasons you do not want to. None of these have any dependencies other than the actual creation of the role. Then there is a separate section of code that is called as a separate function 'dumpUserConfig()' that does other role attributes like 'ALTER ROLE bob SET role TO charlie'. These are the ALTER's that can have dependencies on other roles. I agree this is a little confusing, and if you prefer to see all the ALTER's in one section together directly after the CREATE statements I can make the patch do that, but it isn't necessary to solve the problem. -- 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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
Phil Sorber p...@omniti.com writes: Then there is a separate section of code that is called as a separate function 'dumpUserConfig()' that does other role attributes like 'ALTER ROLE bob SET role TO charlie'. These are the ALTER's that can have dependencies on other roles. Right. Phrased that way, this is an obvious improvement, and I concur that it doesn't look like it could break anything that works now. The more general problem remains to be solved, but we might as well apply this. 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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Thu, Aug 4, 2011 at 2:04 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Aug 4, 2011 at 1:53 PM, Phil Sorber p...@omniti.com wrote: Ok, here is the patch that just moves the ALTER/SET pieces to the end. Can we get this included in the next commit fest? Yep, just make yourself an account and add it. Unfortunately, it doesn't look like anyone ever replied to this thread, but Tom posted some thoughts on another thread that seem to me to be a serious problem for this patch: http://archives.postgresql.org/message-id/13764.1315094...@sss.pgh.pa.us I don't see any easy way around that problem, so I'm going to mark this patch Returned with Feedback for now. It strikes me as craziness to try to guess which settings we should restore at the beginning and which at the end, so I think we need a better idea. I don't really understand why it's not OK to just have pg_dump issue RESET ROLE at appropriate points in the process; that seems like it would be sufficient and not particularly ugly. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
Robert Haas robertmh...@gmail.com writes: I don't really understand why it's not OK to just have pg_dump issue RESET ROLE at appropriate points in the process; that seems like it would be sufficient and not particularly ugly. Well, it was alleged that that would fix this problem: http://archives.postgresql.org/pgsql-hackers/2010-12/msg00916.php but if it does fix it, I think that's a bug in itself: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01031.php But more to the point, I think the specific case of ALTER DATABASE SET ROLE is just one element of a class of problems, namely that settings attached to either databases or roles could create issues for restoring a dump. Issuing RESET ROLE would fix only that one single case. 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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Mon, Oct 10, 2011 at 12:14 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I don't really understand why it's not OK to just have pg_dump issue RESET ROLE at appropriate points in the process; that seems like it would be sufficient and not particularly ugly. Well, it was alleged that that would fix this problem: http://archives.postgresql.org/pgsql-hackers/2010-12/msg00916.php but if it does fix it, I think that's a bug in itself: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01031.php Hmm. But more to the point, I think the specific case of ALTER DATABASE SET ROLE is just one element of a class of problems, namely that settings attached to either databases or roles could create issues for restoring a dump. Issuing RESET ROLE would fix only that one single case. It's not very clear to me that we're going to find a fix that reaches across every setting, though. I mean, for something like maintenance_work_mem, there's no correctness issue regardless of when the new setting takes effect, but there might very well be a performance issue, and it's not really all that clear when the right time to put the old setting back is. And that ambiguity about what's actually correct is, perhaps, the root of the problem. There are related cases where we have a clear-cut policy. For example, we are clear that you must use the newer pg_dump against the older server for best results. That's not always convenient, but at least it's a line in the sand. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Tue, Aug 2, 2011 at 5:05 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Phil Sorber p...@omniti.com writes:
I have included two patches in this email. The first
(dump_user_config_last_with_set_role.patch) is an extension of my
first patch. In addition to moving the ALTER ROLE statements after the
CREATE ROLE statements it also inserts a SET ROLE after every connect.
It takes the role parameter from the --role command line option. This
fixes the problem of not being able to restore to a database because
of lack of permissions. This is similar to the idea proposed here:
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01046.php
I don't understand why you think that that will fix anything?
The problem that Florian originally pointed out is that settings
established by ALTER DATABASE/ROLE could interfere with the restoration
script's actions. That seems to be just as much of a risk for the
--role role as the one originally used to connect. I don't see a way
around that other than not applying those settings until we are done
reconnecting to the target database.
Also, given that the --role switch is only defined to select the role
to be used at *dump* time, I'm unconvinced that forcing it to be used
at *restore* time is a good idea. You'd really need to invent a
separate switch if you were to go down this path.
regards, tom lane
Ok, here is the patch that just moves the ALTER/SET pieces to the end.
Can we get this included in the next commit fest?
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
new file mode 100644
index b5f64e8..d3929f0
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
*** static void dropTablespaces(PGconn *conn
*** 41,48
static void dumpTablespaces(PGconn *conn);
static void dropDBs(PGconn *conn);
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,
--- 41,48
static void dumpTablespaces(PGconn *conn);
static void dropDBs(PGconn *conn);
static void dumpCreateDB(PGconn *conn);
! static void dumpDatabaseConfig(PGconn *conn);
! static void dumpUserConfig(PGconn *conn);
static void dumpDbRoleConfig(PGconn *conn);
static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
const char *type, const char *name, const char *type2,
*** main(int argc, char *argv[])
*** 500,517
/* Dump CREATE DATABASE commands */
if (!globals_only !roles_only !tablespaces_only)
dumpCreateDB(conn);
! /* Dump role/database settings */
! if (!tablespaces_only !roles_only)
{
if (server_version = 9)
dumpDbRoleConfig(conn);
}
}
- if (!globals_only !roles_only !tablespaces_only)
- dumpDatabases(conn);
-
PQfinish(conn);
if (verbose)
--- 500,524
/* Dump CREATE DATABASE commands */
if (!globals_only !roles_only !tablespaces_only)
dumpCreateDB(conn);
+ }
! if (!globals_only !roles_only !tablespaces_only)
! dumpDatabases(conn);
!
! if (!data_only !tablespaces_only server_version = 70300)
! {
! dumpUserConfig(conn);
!
! if (!roles_only)
{
+ if (!globals_only)
+ dumpDatabaseConfig(conn);
+
if (server_version = 9)
dumpDbRoleConfig(conn);
}
}
PQfinish(conn);
if (verbose)
*** dumpRoles(PGconn *conn)
*** 804,812
buf, ROLE, rolename);
fprintf(OPF, %s, buf-data);
-
- if (server_version = 70300)
- dumpUserConfig(conn, rolename);
}
PQclear(res);
--- 811,816
*** dumpCreateDB(PGconn *conn)
*** 1358,1366
fprintf(OPF, %s, buf-data);
- if (server_version = 70300)
- dumpDatabaseConfig(conn, dbname);
-
free(fdbname);
}
--- 1362,1367
*** dumpCreateDB(PGconn *conn)
*** 1375,1418
* Dump database-specific configuration
*/
static void
! dumpDatabaseConfig(PGconn *conn, const char *dbname)
{
! PQExpBuffer buf = createPQExpBuffer();
! int count = 1;
! for (;;)
{
! PGresult *res;
! if (server_version = 9)
! 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 = 9)
! appendPQExpBuffer(buf, ));
! appendPQExpBuffer(buf, ;);
! res = executeQuery(conn, buf-data);
! if (PQntuples(res) == 1
! !PQgetisnull(res, 0, 0))
! {
! makeAlterConfigCommand(conn, PQgetvalue(res, 0,
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Thu, Aug 4, 2011 at 1:53 PM, Phil Sorber p...@omniti.com wrote: Ok, here is the patch that just moves the ALTER/SET pieces to the end. Can we get this included in the next commit fest? Yep, just make yourself an account and add it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
I have included two patches in this email. The first (dump_user_config_last_with_set_role.patch) is an extension of my first patch. In addition to moving the ALTER ROLE statements after the CREATE ROLE statements it also inserts a SET ROLE after every connect. It takes the role parameter from the --role command line option. This fixes the problem of not being able to restore to a database because of lack of permissions. This is similar to the idea proposed here: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01046.php I think this patch is the better one for several reasons. It keeps the ALTER ROLE and ALTER DATABASE statements in the same sections as their related CREATE ROLE and CREATE DATABASE statements, thus not dramatically altering the output of the command. It is a much simpler non-invasive patch. It's concise and easy to understand. It solves all the known scenario's that we have discussed previously. That being said, I understand you have reservations about how it works. You would prefer to move all of these type's of ALTER statements to the end of the output. I also would prefer to see the correct behavior included in the main tree over having my preference over the changes. To that end I have also included a second patch (dump_all_configs_last.patch) which handles it the way you prefer. This creates new sections at the bottom for the ALTER statements. These two patches are mutually exclusive. I would ask that you at least consider the first patch before dismissing the idea. However, I think applying either would would be an acceptable approach. Thanks for your consideration. On Thu, Jul 28, 2011 at 10:06 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jul 27, 2011 at 7:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think pg_dumpall is the very least of your problems if you do something like that. We probably ought to forbid it entirely. Well, we had a long discussion of that on the thread Phil linked to, and I don't think there was any consensus that forbidding it was the right thing to do. You're right, I was half-remembering that thread and thinking that there are a lot of gotchas in doing an ALTER ROLE SET ROLE. Florian claimed in the thread that he'd never hit one before, but that doesn't convince me much. Phil appears to be trying to implement the proposal you made here: http://archives.postgresql.org/pgsql-hackers/2011-01/msg00452.php ...although I don't think that what he did quite matches what you asked for. No, the proposed patch doesn't go nearly far enough to address Florian's problem. What I was speculating about was moving all the role (and database) alters to the *end*, so they'd not take effect until after we'd completed all the restore actions. regards, tom lane diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c new file mode 100644 index b5f64e8..7f2b03e *** a/src/bin/pg_dump/pg_dumpall.c --- b/src/bin/pg_dump/pg_dumpall.c *** static int server_version; *** 79,84 --- 79,85 static FILE *OPF; static char *filename = NULL; + static char *use_role = NULL; int main(int argc, char *argv[]) *** main(int argc, char *argv[]) *** 87,93 char *pgport = NULL; char *pguser = NULL; char *pgdb = NULL; - char *use_role = NULL; enum trivalue prompt_password = TRI_DEFAULT; bool data_only = false; bool globals_only = false; --- 88,93 *** main(int argc, char *argv[]) *** 443,448 --- 443,452 fprintf(OPF, \\connect postgres\n\n); + if (use_role server_version = 80100) + fprintf(OPF, SET ROLE %s;\n, + fmtId(use_role)); + /* Replicate encoding and std_strings in output */ fprintf(OPF, SET client_encoding = '%s';\n, pg_encoding_to_char(encoding)); *** dumpRoles(PGconn *conn) *** 804,814 buf, ROLE, rolename); fprintf(OPF, %s, buf-data); - - if (server_version = 70300) - dumpUserConfig(conn, rolename); } PQclear(res); fprintf(OPF, \n\n); --- 808,819 buf, ROLE, rolename); fprintf(OPF, %s, buf-data); } + if (server_version = 70300) + for (i = 0; i PQntuples(res); i++) + dumpUserConfig(conn, PQgetvalue(res, i, i_rolname)); + PQclear(res); fprintf(OPF, \n\n); *** dumpDatabases(PGconn *conn) *** 1561,1566 --- 1566,1575 fprintf(OPF, \\connect %s\n\n, fmtId(dbname)); + if (use_role server_version = 80100) + fprintf(OPF, SET ROLE %s;\n\n, + fmtId(use_role)); + if (filename) fclose(OPF); diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c new file mode 100644 index b5f64e8..d3929f0 *** a/src/bin/pg_dump/pg_dumpall.c --- b/src/bin/pg_dump/pg_dumpall.c *** static void dropTablespaces(PGconn *conn *** 41,48 static void dumpTablespaces(PGconn *conn); static void
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
Phil Sorber p...@omniti.com writes: I have included two patches in this email. The first (dump_user_config_last_with_set_role.patch) is an extension of my first patch. In addition to moving the ALTER ROLE statements after the CREATE ROLE statements it also inserts a SET ROLE after every connect. It takes the role parameter from the --role command line option. This fixes the problem of not being able to restore to a database because of lack of permissions. This is similar to the idea proposed here: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01046.php I don't understand why you think that that will fix anything? The problem that Florian originally pointed out is that settings established by ALTER DATABASE/ROLE could interfere with the restoration script's actions. That seems to be just as much of a risk for the --role role as the one originally used to connect. I don't see a way around that other than not applying those settings until we are done reconnecting to the target database. Also, given that the --role switch is only defined to select the role to be used at *dump* time, I'm unconvinced that forcing it to be used at *restore* time is a good idea. You'd really need to invent a separate switch if you were to go down this path. 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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Jul 27, 2011 at 7:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: Phil Sorber p...@omniti.com writes: Currently if you use 'ALTER ROLE rolename SET ROLE', pg_dumpall will dump an 'ALTER ROLE' out right after the 'CREATE ROLE' statement. I think pg_dumpall is the very least of your problems if you do something like that. We probably ought to forbid it entirely. Well, we had a long discussion of that on the thread Phil linked to, and I don't think there was any consensus that forbidding it was the right thing to do. Phil appears to be trying to implement the proposal you made here: http://archives.postgresql.org/pgsql-hackers/2011-01/msg00452.php ...although I don't think that what he did quite matches what you asked for. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
Robert Haas robertmh...@gmail.com writes: On Wed, Jul 27, 2011 at 7:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think pg_dumpall is the very least of your problems if you do something like that. We probably ought to forbid it entirely. Well, we had a long discussion of that on the thread Phil linked to, and I don't think there was any consensus that forbidding it was the right thing to do. You're right, I was half-remembering that thread and thinking that there are a lot of gotchas in doing an ALTER ROLE SET ROLE. Florian claimed in the thread that he'd never hit one before, but that doesn't convince me much. Phil appears to be trying to implement the proposal you made here: http://archives.postgresql.org/pgsql-hackers/2011-01/msg00452.php ...although I don't think that what he did quite matches what you asked for. No, the proposed patch doesn't go nearly far enough to address Florian's problem. What I was speculating about was moving all the role (and database) alters to the *end*, so they'd not take effect until after we'd completed all the restore actions. 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] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
Phil Sorber p...@omniti.com writes: Currently if you use 'ALTER ROLE rolename SET ROLE', pg_dumpall will dump an 'ALTER ROLE' out right after the 'CREATE ROLE' statement. I think pg_dumpall is the very least of your problems if you do something like that. We probably ought to forbid it entirely. 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
