Re: Role Attribute Bitmask Catalog Representation

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Alvaro Herrera wrote:
> > I think we should create a new header file (maybe acltypes.h or
> > acldefs.h), which only contains the AclMode and RoleAttr typedefs and
> > their associated defines; that one would be included from parsenodes.h,
> > acl.h and pg_authid.h. Everything else would be in acl.h. So code that
> > currently checks permissions using only acl.h do not need any extra
> > includes.
>
> I propose this patch on top of Adam's v5. Also included is a full patch
> against master.

Thanks! I've just read through your changes to Adam's v5 and they all
look reasonable to me. I agree that having acldefs.h with the #define's
is nicer and cleaner and reduces the amount of including needed for
pg_authid. I also like that it removes those ACL_X definitions from
parsenodes.h.

Thanks also for the whiteline/line-wrap improvements and user.c cleanup,
nice that we don't need all of those individual variables now that we're
using a bitmask.

> Unrelated: when changing from unified to context format, I saw
> filterdiff fail to produce a complete patch, skipping some hunks in its
> output. My first impression was that I had dropped some hunks in git,
> so I wasted some time comparing v5 and v6 by hand before remembering
> that Michael Paquier had mentioned this problem previously.

Ugh, that's definitely frustrating.. Will keep it in mind.

Thanks again!

Stephen