On Wed, Sep 3, 2014 at 10:40 AM, Stephen Frost <sfr...@snowman.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 -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers