Re: [COMMITTERS] pgsql: Use a bitmask to represent role attributes - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: [COMMITTERS] pgsql: Use a bitmask to represent role attributes
Date
Msg-id 20141223153447.GD3062@tamriel.snowman.net
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Use a bitmask to represent role attributes  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [COMMITTERS] pgsql: Use a bitmask to represent role attributes  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > Use a bitmask to represent role attributes
> > The previous representation using a boolean column for each attribute
> > would not scale as well as we want to add further attributes.
>
> > Extra auxilliary functions are added to go along with this change, to
> > make up for the lost convenience of access of the old representation.
>
> I have to apologize for not having paid more attention, but ... is this
> *really* such a great idea?  You've just broken any client-side code
> that looks directly at pg_authid.

Anything which looks at pg_authid for the relevant columns needs to be
updated for the changes which were made for rolreplication previously,
and now rolbypassrls and any other role attributes added.  While I agree
that it's something to consider (and even mentioned it earlier in the
thread..), there wasn't any argument about it from those who were
following the discussion.

Perhaps we could have gone out of our way to ask the pgAdmin authors and
other client-side utilities which look at these columns if this will
more issues than new columns do.

> Moreover, I don't particularly buy
> the idea that this somehow insulates us from the compatibility costs of
> adding new role properties: you're still going to have to add columns to
> the pg_roles view, and adjust clients that look at that, every time.

That's correct, however, this change wasn't intended to insulate anyone
from those compatibility changes but rather to make better use of the
bytes in each pg_authid record.  We were already up to 8 individual bool
columns, wasting space for each.

> Replacing bool-column accesses with bitmask manipulation doesn't seem
> like it's a win on a micro-optimization level either, certainly not for
> SQL-level coding where you've probably made it two orders of magnitude
> more expensive.

I agree that it's more expensive for SQL users, but it's not our first
use of a bitmask in the catalog.  Perhaps this is more user-visible than
other use-cases but we also provide views to address common usages in
this case already, where we don't in other situations.

> And lastly, what happens when you run out of bits in
> that bigint column?

We can add another, though this seems unlikely to happen given the
previous discussion and review of what additional role attributes would
be nice to add.  There are some scenarios I've considered which would
lead to using perhaps another 15-20, but 64 sure seems far off.

> Again, I suppose I should have objected earlier, but I really seriously
> doubt that this is a good idea.

I tried to solicit discussion about these concerns earlier but we're all
busy, no problem.

For my 2c, I do think this is the right direction to go in as adding
another 15 boolean columns to pg_authid definitely strikes me as a bad
idea, but we can certainly discuss the trade-offs.  Another proposal
(from Simon, as I recall) was to use an array.  That runs into nearly
all the same problems and the on-disk representation ends up being even
larger, which is why I didn't see that being a good idea.
Thanks,
    Stephen

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Proposal "VACUUM SCHEMA"
Next
From: Tom Lane
Date:
Subject: Re: [COMMITTERS] pgsql: Use a bitmask to represent role attributes