Re: Re: Segmentation fault in pg_dumpall from master down to 9.1 and other bug introduced by RLS - Mailing list pgsql-hackers

* Noah Misch (noah@leadboat.com) wrote:
> On Fri, Nov 14, 2014 at 11:47:49AM -0500, Stephen Frost wrote:
> > rolcreaterole uses usesuper, while rolreplication and rolbypassrls do
> > not.  Noah- would you argue that we should change rolcreaterole, which
> > has this behavior in all released branches (though, of course, it's only
> > relevant when upgrading from a pre-8.1 server where we didn't have
> > rolcreaterole)?
>
> Setting aside that I wouldn't have argued for any change here, yes.  I agree
> that there's no good reason to handle rolcreaterole unlike rolreplication.
> The choice between their respective techniques has behavior consequences only
> if you later use ALTER ROLE x NOSUPERUSER, which I have not seen done apart
> from explicit ALTER ROLE testing.  Having said that, I prefer setting these
> attributes to false when dumping from a version that did not have them, for
> these reasons:
>
> 1) It's fail-safe.  The hypothetical ALTER ROLE x NOSUPERUSER may leave the
>    role with fewer privileges than expected, never with more privileges than
>    expected.
>
> 2) It's more consistent with how folks create superuser accounts.  I've not
>    seen "CREATE USER x SUPERUSER CREATEROLE" used.  Where SUPERUSER preempts a
>    given role attribute, the norm among sites I've seen is to omit the
>    attribute.  (The bootstrap superuser does turn this point on its head.)
>
> 3) It's cleaner in \du output.

I agree with these points and my experience matches yours.  The
bootstrap user is the one anomaly at those sites and makes it stand out
rather than look like the majority of the superuser accounts which
exist (where there exists more than one or two anyway).

> I can't pinpoint a technical argument against your proposal to cease adding
> excess attributes to the bootstrap superuser at initdb time.  It feels like a
> case of the tail wagging the dog, changing antediluvian initdb behavior to
> make pg_dumpall slightly more transparent.

For my 2c, it feels like it was simply an attempt to reflect actual
capabilities without consideration for what happened later rather than
any particularly technical reason, and I don't feel that original
reasoning (if that's what it was, anyway) was sound.

> So, if you desire to make this consistent, I recommend using rolreplication's
> treatment as the gold standard.  That is to say, when dumping from an older
> version, set to false any of these role attributes not existing in that
> version.  Robert's proposal to emit neither NOBYPASSRLS nor BYPASSRLS is a
> reasonable alternative, though less so for "pg_dumpall --clean".  It would be
> defensible to send NOBYPASSRLS under --clean and omit the option otherwise;
> consider that my second choice.

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.

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.

> > What are your thoughts on the additional role
> > attributes which are being discussed?
>
> All three of rolcreaterole, rolreplication and rolbypassrls deserve the same
> dumpRoles() treatment.

Agreed.
Thanks!
    Stephen