Re: Re: Segmentation fault in pg_dumpall from master down to 9.1 and other bug introduced by RLS

* Noah Misch (noah@leadboat.com) wrote:
> On Fri, Nov 14, 2014 at 08:39:28PM -0500, Stephen Frost wrote:
> > I don't see the point in including them for --clean..?  --clean states
> > that DROP commands would be added, not that existing roles would be
> > adjusted in some way.
>
> It does state that, but note this comment in dumpRoles():
>
>         /*
>          * We dump CREATE ROLE followed by ALTER ROLE to ensure that the role
>          * will acquire the right properties even if it already exists (ie, it
>          * won't hurt for the CREATE to fail).  This is particularly important
>          * for the role we are connected as, since even with --clean we will
>          * have failed to drop it.  binary_upgrade cannot generate any errors,
>          * so we assume the current role is already created.
>          */

Ah, yes, of course.

> Under --clean, "the right properties" are those the role would have had if the
> DROP ROLE had succeeded.  Those are necessarily independent of the pre-DROP
> version of the role.  (Otherwise, you potentially get different outcomes
> depending on which superuser restored the --clean dump.)

Agreed, and in this case we'd need to set any attributes not set back to
the default, which would include having NOBYPASSRLS.

> > As for using 'always false'- I tend to think Robert actually has it
> > better by using the default for users.  Consider rolinherit- that
> > defaults to 'true' and while it would technically be more 'safe' to set
> > it to false, it wouldn't have matched what we provided under the user /
> > group system prior to roles.  Doing this would also reduce clutter in
> > pg_dumpall output.
>
> My arguments and conclusion apply only to the permission-like attributes that
> are subsets of SUPERUSER.  rolinherit is indeed not in that category.

Understood.
Thanks!
    Stephen