commit f9a087ad4a89e3ffad7c4bd196bcf89b7e496a72 Author: Nathan Bossart 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,