pg.ddx.io pgsql-bugs@postgresql.org mailing list archive help / color / mirror / Atom feed From: Bossart, Nathan <bossartn@amazon.com>
To: pgsql-bugs@postgresql.org <pgsql-bugs@postgresql.org>
Subject: Re: BUG #14242: Role with a setconfig "role" setting to a nonexistent role causes pg_upgrade to fail
Date: Mon, 11 Jul 2016 22:48:08 +0000
Message-ID: <5e6af34f945e4ddab2caf2a26fa57137@EX13D03UWC002.ant.amazon.com> (raw )
In-Reply-To: <20160711223641.1426.86096@wrigleys.postgresql.org >
References: <20160711223641.1426.86096@wrigleys.postgresql.org >
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-bugs>
A simple fix for this issue would be to temporarily create the role just for the ALTER ROLE SET command generated by pg_dumpall. I've attached a patch that does this.
Nathan
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Attachments:
restore_invalid_setconfig.patch (application/octet-stream, 10.9K) download
commit f9a087ad4a89e3ffad7c4bd196bcf89b7e496a72
Author: Nathan Bossart <bossartn@amazon.com>
Date: Mon Jul 11 18:47:16 2016 +0000
Create temporary role during pg_dumpall for roles with a setconfig
"role" setting to a nonexistent role.
It is possible to modify the "role" setting in setconfig in the
pg_db_role_setting table such that it points to a nonexistent role.
When this is the case, restoring the output of pg_dumpall will fail
due to the missing role:
ERROR: role "foo" does not exist
A notable place where this causes issues is during pg_upgrade. As
soon as pg_upgrade hits this error it fails.
This change prevents this error by creating the nonexistent role
temporarily for the generated ALTER ROLE SET statement that would
otherwise fail. This results in pg_dumpall output similar to the
following:
CREATE ROLE foo;
ALTER ROLE bar SET role TO 'foo';
DROP ROLE IF EXISTS foo;
This issue was identified by Jordan Lange and Nathan Bossart, and
this patch fixes bug #14242. Back-patch to all supported versions.
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index d4fb03e..15479b8 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -45,6 +45,7 @@ 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 bool roleExists(const char *role_name, PGconn *conn);
static void makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
const char *type, const char *name, const char *type2,
const char *name2);
@@ -1624,6 +1625,29 @@ dumpDbRoleConfig(PGconn *conn)
/*
+ * Helper function for makeAlterConfigCommand(...) to determine if a role exists.
+ */
+static bool
+roleExists(const char *role_name, PGconn *conn)
+{
+ PGresult *res;
+ bool role_exists;
+ PQExpBuffer query = createPQExpBuffer();
+
+ appendPQExpBuffer(query, "SELECT 1 FROM pg_roles WHERE rolname=");
+ appendStringLiteralConn(query, role_name, conn);
+ appendPQExpBuffer(query, ";");
+
+ res = executeQuery(conn, query->data);
+ destroyPQExpBuffer(query);
+
+ role_exists = PQntuples(res) > 0;
+ PQclear(res);
+ return role_exists;
+}
+
+
+/*
* Helper function for dumpXXXConfig().
*/
static void
@@ -1634,6 +1658,7 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
char *pos;
char *mine;
PQExpBuffer buf;
+ bool temp_role_used = false;
mine = pg_strdup(arrayitem);
pos = strchr(mine, '=');
@@ -1646,6 +1671,21 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
buf = createPQExpBuffer();
*pos = 0;
+
+ /*
+ * If a role's setconfig contains {role="foo"} and the role "foo"
+ * does not exist, the ALTER ROLE SET command generated here will
+ * fail. In order to prevent this error, the nonexistent role is
+ * temporarily created just for the invalid ALTER ROLE SET command.
+ */
+ if (pg_strcasecmp(type, "ROLE") == 0
+ && pg_strcasecmp(mine, "ROLE") == 0
+ && !roleExists(pos + 1, conn))
+ {
+ temp_role_used = true;
+ appendPQExpBuffer(buf, "CREATE ROLE %s;\n", fmtId(pos + 1));
+ }
+
appendPQExpBuffer(buf, "ALTER %s %s ", type, fmtId(name));
if (type2 != NULL && name2 != NULL)
appendPQExpBuffer(buf, "IN %s %s ", type2, fmtId(name2));
@@ -1661,6 +1701,14 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
appendStringLiteralConn(buf, pos + 1, conn);
appendPQExpBufferStr(buf, ";\n");
+ /*
+ * If a temporary role was needed in order to prevent an invalid
+ * ALTER ROLE SET command from failing, it is deleted immediately
+ * after the ALTER ROLE SET command is executed.
+ */
+ if (temp_role_used)
+ appendPQExpBuffer(buf, "DROP ROLE IF EXISTS %s;\n", fmtId(pos + 1));
+
fprintf(OPF, "%s", buf->data);
destroyPQExpBuffer(buf);
free(mine);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index fd4b9e8..5f4d0ba 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -277,6 +277,58 @@ my %tests = (
section_pre_data => 1,
section_post_data => 1,
test_schema_plus_blobs => 1, }, },
+ 'ALTER ROLE invalid_setconfig_test' => {
+ regexp => qr/^
+ \QALTER ROLE invalid_setconfig_test WITH \E
+ \QNOSUPERUSER INHERIT NOCREATEROLE NOCREATEDB NOLOGIN \E
+ \QNOREPLICATION NOBYPASSRLS;\E
+ /xm,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
+ 'ALTER ROLE valid_setconfig_test' => {
+ regexp => qr/^
+ \QALTER ROLE valid_setconfig_test WITH \E
+ \QNOSUPERUSER INHERIT NOCREATEROLE NOCREATEDB NOLOGIN \E
+ \QNOREPLICATION NOBYPASSRLS;\E
+ /xm,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
'ALTER FUNCTION dump_test.pltestlang_call_handler() OWNER TO' => {
regexp => qr/^
\QALTER FUNCTION dump_test.pltestlang_call_handler() \E
@@ -835,6 +887,110 @@ my %tests = (
section_pre_data => 1,
section_post_data => 1,
test_schema_plus_blobs => 1, }, },
+ 'CREATE ROLE invalid_setconfig_test' => {
+ create_order => 2,
+ create_sql => 'CREATE ROLE invalid_setconfig_test;',
+ regexp => qr/^CREATE ROLE invalid_setconfig_test;/m,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
+ 'CREATE ROLE valid_setconfig_test' => {
+ create_order => 3,
+ create_sql => 'CREATE ROLE valid_setconfig_test;',
+ regexp => qr/^CREATE ROLE valid_setconfig_test;/m,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
+ 'ALTER ROLE invalid_setconfig_test SET role=\'nonexistent_role\'' => {
+ create_sql => 'CREATE ROLE nonexistent_role;
+ ALTER ROLE invalid_setconfig_test SET role=\'nonexistent_role\';
+ DROP ROLE nonexistent_role;',
+ regexp => qr/^
+ \QCREATE ROLE nonexistent_role;\E
+ \n\QALTER ROLE invalid_setconfig_test SET role TO 'nonexistent_role';\E
+ \n\QDROP ROLE IF EXISTS nonexistent_role;\E
+ /xm,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
+ 'ALTER ROLE valid_setconfig_test SET role=\'valid_setconfig_test\'' => {
+ create_sql => 'ALTER ROLE valid_setconfig_test SET role=\'valid_setconfig_test\';',
+ regexp => qr/^
+ (?!\QCREATE ROLE valid_setconfig_test;\E\n)
+ \QALTER ROLE valid_setconfig_test SET role TO 'valid_setconfig_test';\E
+ (?!\n\QDROP ROLE IF EXISTS valid_setconfig_test;\E)
+ /xm,
+ like => {
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1, },
+ unlike => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ exclude_test_table_data => 1,
+ no_privs => 1,
+ no_owner => 1,
+ only_dump_test_schema => 1,
+ only_dump_test_table => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ section_post_data => 1,
+ test_schema_plus_blobs => 1, }, },
'CREATE DATABASE postgres' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -2006,7 +2162,7 @@ my %tests = (
clean_if_exists => 1,
pg_dumpall_globals_clean => 1, }, },
'DROP ... commands' => { # catch-all for DROP
- regexp => qr/^DROP /m,
+ regexp => qr/^DROP (?!ROLE IF EXISTS nonexistent_role;)/m,
like => {}, # use more-specific options above
unlike => {
binary_upgrade => 1,
view thread (14+ messages) latest in thread
Message-ID: <5e6af34f945e4ddab2caf2a26fa57137@EX13D03UWC002.ant.amazon.com>
Permalink: ../5e6af34f945e4ddab2caf2a26fa57137@EX13D03UWC002.ant.amazon.com/
Also on: postgresql.org/message-id/5e6af34f945e4ddab2caf2a26fa57137@EX13D03UWC002.ant.amazon.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: bossartn@amazon.com
Subject: Re: BUG #14242: Role with a setconfig "role" setting to a nonexistent role causes pg_upgrade to fail
In-Reply-To: <5e6af34f945e4ddab2caf2a26fa57137@EX13D03UWC002.ant.amazon.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox