Re: Role Attribute Bitmask Catalog Representation

Adam,

* Adam Brightwell (adam(dot)brightwell(at)crunchydatasolutions(dot)com) wrote:
> Ok. Though, this would affect how CATUPDATE is handled. Peter Eisentraut
> previously raised a question about whether superuser checks should be
> included with catupdate which led me to create the following post.
>
> http://www.postgresql.org/message-id/CAKRt6CQOvT2KiyKg2gFf7h9k8+JVU1149zLb0EXTkKk7TAQGMQ@mail.gmail.com
>
> Certainly, we could keep has_rolcatupdate for this case and put the
> superuser check in role_has_attribute, but it seems like it might be worth
> taking a look at whether a superuser can bypass catupdate or not. Just a
> thought.

My recollection matches the documentation- rolcatupdate should be
required to update the catalogs. The fact that rolcatupdate is set by
AlterUser to match rolsuper is an interesting point and one which we
might want to reconsider, but that's beyond the scope of this patch.

Ergo, I'd suggest keeping has_rolcatupdate, but have it do the check
itself directly instead of calling down into role_has_attribute().

There's an interesting flip side to that though, which is the question
of what to do with pg_roles and psql. Based on the discussion this far,
it seems like we'd want to keep the distinction for pg_roles and psql
based on what bits have explicitly been set rather than what's actually
checked for. As such, I'd have one other function-
check_has_attribute() which *doesn't* have the superuser allow-all check
and is what is used in pg_roles and by psql. I'd expose both functions
at the SQL level.

> Ok. I had originally thought for this patch that I would try to minimize
> these types of changes, though perhaps this is that opportunity previously
> mentioned in refactoring those. However, the catupdate question still
> remains.

It makes sense to me, at least, to include removing those individual
attribute functions in this patch.

> I have no reason for one over the other, though I did ask myself that
> question. I did find it curious that in some cases there is "has_X" and
> then in others "pg_has_X". Perhaps I'm not looking in the right places,
> but I haven't found anything that helps to distinguish when one vs the
> other is appropriate (even if it is a general rule of thumb).

Given that we're changing things anyway, it seems to me that the pg_
prefix makes sense.

> Yes, we were, however the latter causes a syntax error with initdb. :-/

Ok, then just stuff the 255 back there and add a comment about why it's
required and mention that cute tricks to calculate the value won't work.

Thanks!

Stephen