Error during restore - dump taken with pg_dumpall -c option

Started by Rushabh Lathiaover 9 years ago6 messages
#1Rushabh Lathia
rushabh.lathia@gmail.com
1 attachment(s)

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

dumpRoles()::pg_dumpall.c does have logic to not dump "CREATE ROLE" if the
rolename starts with "pg_", but similar check is missing into dropRoles()
function.

PFA patch, to fix the problem in the similar way its been handled into
dumpRoles().

Thanks,

--
Rushabh Lathia
www.EnterpriseDB.com

Attachments:

pgdump_droprole_fix.patchapplication/x-patch; name=pgdump_droprole_fix.patchDownload
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 694bd1e..fe1d8ba 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -605,7 +605,13 @@ dropRoles(PGconn *conn)
 	int			i_rolname;
 	int			i;
 
-	if (server_version >= 80100)
+	if (server_version >= 90600)
+		res = executeQuery(conn,
+						   "SELECT rolname "
+						   "FROM pg_authid "
+						   "WHERE rolname !~ '^pg_' "
+						   "ORDER BY 1");
+	else if (server_version >= 80100)
 		res = executeQuery(conn,
 						   "SELECT rolname "
 						   "FROM pg_authid "
@@ -630,6 +636,13 @@ dropRoles(PGconn *conn)
 
 		rolename = PQgetvalue(res, i, i_rolname);
 
+		if (strncmp(rolename,"pg_",3) == 0)
+		{
+			fprintf(stderr, _("%s: role name starting with \"pg_\" skipped (%s)\n"),
+					progname, rolename);
+			continue;
+		}
+
 		fprintf(OPF, "DROP ROLE %s%s;\n",
 				if_exists ? "IF EXISTS " : "",
 				fmtId(rolename));
#2Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Rushabh Lathia (#1)
Re: Error during restore - dump taken with pg_dumpall -c option

Em quinta-feira, 12 de maio de 2016, Rushabh Lathia <
rushabh.lathia@gmail.com> escreveu:

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

dumpRoles()::pg_dumpall.c does have logic to not dump "CREATE ROLE" if the
rolename starts with "pg_", but similar check is missing into dropRoles()
function.

PFA patch, to fix the problem in the similar way its been handled into
dumpRoles().

Shouldn't this logic be applied just to version >= 9.6? I ask it because
you write a special query filtering rolname !~ '^pg_' and again check it
using strcmp before the drop role output. Is this the expected behavior?

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello

#3Michael Paquier
michael.paquier@gmail.com
In reply to: Fabrízio de Royes Mello (#2)
Re: Error during restore - dump taken with pg_dumpall -c option

On Thu, May 12, 2016 at 9:48 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

Em quinta-feira, 12 de maio de 2016, Rushabh Lathia
<rushabh.lathia@gmail.com> escreveu:

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

dumpRoles()::pg_dumpall.c does have logic to not dump "CREATE ROLE" if
the
rolename starts with "pg_", but similar check is missing into dropRoles()
function.

PFA patch, to fix the problem in the similar way its been handled into
dumpRoles().

Shouldn't this logic be applied just to version >= 9.6? I ask it because you
write a special query filtering rolname !~ '^pg_' and again check it using
strcmp before the drop role output. Is this the expected behavior?

The patch looks correct to me: as far as I recall we give no guarantee
that a dump generated by pg_dump based on a given version would load
data correctly in an older version of the backend. So, when with 9.6's
pg_dump, dumping from a < 9.6 backend, bypassing the role names
beginning by "pg_" and letting the user know about their existence
without dumping them looks fine.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Stephen Frost
sfrost@snowman.net
In reply to: Rushabh Lathia (#1)
Re: Error during restore - dump taken with pg_dumpall -c option

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

Ah, yes, dropRoles() needs the same change that dumpRoles() got.

Will fix, thanks!

Stephen

#5Rushabh Lathia
rushabh.lathia@gmail.com
In reply to: Stephen Frost (#4)
Re: Error during restore - dump taken with pg_dumpall -c option

On Fri, May 13, 2016 at 7:27 PM, Stephen Frost <sfrost@snowman.net> wrote:

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

Ah, yes, dropRoles() needs the same change that dumpRoles() got.

Will fix, thanks!

Thanks Stephen.

#6Stephen Frost
sfrost@snowman.net
In reply to: Rushabh Lathia (#5)
Re: Error during restore - dump taken with pg_dumpall -c option

Rushabh,

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

On Fri, May 13, 2016 at 7:27 PM, Stephen Frost <sfrost@snowman.net> wrote:

* Rushabh Lathia (rushabh.lathia@gmail.com) wrote:

On master branch when we do pg_dumpall with -c option, I can see that
it also dumping the "DROP ROLE pg_signal_backend", which seems wrong.
Because when you restore the dump, its throwing an error
"ERROR: cannot drop role pg_signal_backend because it is required by the
database system".

Ah, yes, dropRoles() needs the same change that dumpRoles() got.

Will fix, thanks!

Thanks Stephen.

I've puhsed a fix for this.

Thanks!

Stephen