Re: [HACKERS] Error during restore - dump taken with pg_dumpall -c option

2016-05-24 Thread Stephen Frost
Rushabh,

* Rushabh Lathia (rushabh.lat...@gmail.com) wrote:
> On Fri, May 13, 2016 at 7:27 PM, Stephen Frost  wrote:
> 
> > * Rushabh Lathia (rushabh.lat...@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


signature.asc
Description: Digital signature


Re: [HACKERS] Error during restore - dump taken with pg_dumpall -c option

2016-05-21 Thread Rushabh Lathia
On Fri, May 13, 2016 at 7:27 PM, Stephen Frost  wrote:

> * Rushabh Lathia (rushabh.lat...@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.


Re: [HACKERS] Error during restore - dump taken with pg_dumpall -c option

2016-05-13 Thread Stephen Frost
* Rushabh Lathia (rushabh.lat...@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


signature.asc
Description: Digital signature


Re: [HACKERS] Error during restore - dump taken with pg_dumpall -c option

2016-05-13 Thread Michael Paquier
On Thu, May 12, 2016 at 9:48 PM, Fabrízio de Royes Mello
 wrote:
>
>
> Em quinta-feira, 12 de maio de 2016, Rushabh Lathia
>  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


Re: [HACKERS] Error during restore - dump taken with pg_dumpall -c option

2016-05-12 Thread Fabrízio de Royes Mello
Em quinta-feira, 12 de maio de 2016, Rushabh Lathia <
rushabh.lat...@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
>> 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