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 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 >= 90000)
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 >= 90000)
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 >= 90000)
! 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 >= 90000)
! 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++;
! }
! else
! {
! PQclear(res);
! break;
}
}
! destroyPQExpBuffer(buf);
}
--- 1376,1450 ----
* Dump database-specific configuration
*/
static void
! dumpDatabaseConfig(PGconn *conn)
{
! PGresult *dbres;
! int i;
! bool shown_header = false;
! if (server_version >= 70100)
! dbres = executeQuery(conn,
! "SELECT datname "
! "FROM pg_database d "
! "WHERE datallowconn ORDER BY 1");
! else
! dbres = executeQuery(conn,
! "SELECT datname "
! "FROM pg_database d "
! "ORDER BY 1");
!
! for (i = 0; i < PQntuples(dbres); i++)
{
! char *dbname = PQgetvalue(dbres, i, 0);
! PQExpBuffer buf = createPQExpBuffer();
! int count = 1;
! for (;;)
! {
! PGresult *res;
! if (server_version >= 90000)
! 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 >= 90000)
! appendPQExpBuffer(buf, ")");
!
! appendPQExpBuffer(buf, ";");
!
! res = executeQuery(conn, buf->data);
! if (PQntuples(res) == 1 &&
! !PQgetisnull(res, 0, 0))
! {
! if (!shown_header)
! {
! shown_header = true;
! fprintf(OPF, "--\n-- Database Config\n--\n\n");
! }
!
! makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "DATABASE", dbname, NULL, NULL);
! PQclear(res);
! count++;
! }
! else
! {
! PQclear(res);
! break;
! }
}
+
+ destroyPQExpBuffer(buf);
}
! PQclear(dbres);
!
! if (shown_header)
! fprintf(OPF, "\n\n");
}
*************** dumpDatabaseConfig(PGconn *conn, const c
*** 1421,1464 ****
* Dump user-specific configuration
*/
static void
! dumpUserConfig(PGconn *conn, const char *username)
{
! PQExpBuffer buf = createPQExpBuffer();
! int count = 1;
! for (;;)
{
! PGresult *res;
! if (server_version >= 90000)
! 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 >= 90000)
! 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++;
! }
! else
{
! PQclear(res);
! break;
}
}
! destroyPQExpBuffer(buf);
}
--- 1453,1535 ----
* Dump user-specific configuration
*/
static void
! dumpUserConfig(PGconn *conn)
{
! PGresult *userres;
! int i_rolname;
! int i;
! bool shown_header = false;
! if (server_version >= 80100)
! userres = executeQuery(conn,
! "SELECT rolname "
! "FROM pg_authid "
! "ORDER BY 1");
! else
! userres = executeQuery(conn,
! "SELECT usename as rolname "
! "FROM pg_shadow "
! "UNION "
! "SELECT groname as rolname "
! "FROM pg_group "
! "ORDER BY 1");
!
! i_rolname = PQfnumber(userres, "rolname");
!
! for (i = 0; i < PQntuples(userres); i++)
{
! const char *username;
! username = PQgetvalue(userres, i, i_rolname);
! PQExpBuffer buf = createPQExpBuffer();
! int count = 1;
!
! for (;;)
{
! PGresult *res;
!
! if (server_version >= 90000)
! 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 >= 90000)
! appendPQExpBuffer(buf, ")");
!
! res = executeQuery(conn, buf->data);
! if (PQntuples(res) == 1 &&
! !PQgetisnull(res, 0, 0))
! {
! if (!shown_header)
! {
! shown_header = true;
! fprintf(OPF, "--\n-- Role Config\n--\n\n");
! }
!
! makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
! "ROLE", username, NULL, NULL);
! PQclear(res);
! count++;
! }
! else
! {
! PQclear(res);
! break;
! }
}
+
+ destroyPQExpBuffer(buf);
}
! PQclear(userres);
!
! if (shown_header)
! 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
