Re: RLS Design

On Wed, Sep 3, 2014 at 10:40 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> This is not a full review of this patch; as we're mid-CommitFest, I
>> assume this will get added to the next CommitFest.
>
> As per usual, the expectation is that the patch is reviewed and updated
> during the commitfest. Given that the commitfest isn't even over according
> to the calendar it seems a bit premature to talk about the next one, but
> certainly if it's not up to a commitable level before the end of this
> commitfest then it'll be submitted for the next.

The first version of this patch that was described as "ready for
review" was submitted on August 29th. The previous revision was
submitted on August 18th. Both of those dates are after the
CommitFest deadline of August 15th. So from where I sit this is not
timely submitted for this CommitFest. The last version before August
was submitted in April (there's a link to a version supposedly
submitted in June in the CommitFest application, but it doesn't point
to an email with a patch attached). I don't want to (and don't feel I
should have to) decide between dropping everything to review an
untimely-submitted patch and having it get committed with no review
from anyone who wasn't involved in writing it.

>> + if (AuthenticatedUserIsSuperuser)
>> + SetConfigOption("row_security", "off", PGC_INTERNAL,
>> PGC_S_OVERRIDE);
>>
>> Injecting this kind of magic into InitializeSessionUserId(),
>> SetSessionAuthorization(), and SetCurrentRoleId() seems 100%
>> unacceptable to me.
>
> I was struggling with the right way to address this and welcome suggestions.
> The primary issue is that I really want to support a superuser turning it
> on, so we can't simply have it disabled for all superusers all the time. The
> requirement that it not be enabled by default for superusers makes sense,
> but how far does that extend and how do we address upgrades? In particular,
> can we simply set row_security=off as a custom GUC setting when superusers
> are created or roles altered to be made superusers? Would we do that in
> pg_upgrade?

I think you need to have the GUC have one default value, not one
default for superusers and another default for everybody else. I
previously proposed making the GUC on/off/force, with "on" meaning
"apply row-level security unless we have permission to bypass it,
either because we are the table owner or the superuser", "off" meaning
"error out if we would be forced to apply row-level security", and
"force" meaning "always apply row-level security even if we have
permission to bypass it". I still think that's a good proposal.
There may be other reasonable alternatives as well, but making changes
to one GUC magically change other GUCs under the hood isn't one of
them.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company