> I've attached a patch to implement it.  It's not fully polished but it's
> sufficient for comment, I believe.  Additional comments, documentation
> and regression tests are to be added, if we have agreement on the
> grammer and implementation approach.

I have given the first patch a quick review.  I think I agree with the
grammar, it makes sense to me.  At first I didn't really like NO
FORCE, but I couldn't think of anything better.

One comment:

  if (pg_class_ownercheck(relid, user_id))
- return RLS_NONE_ENV;
+ {
+ if (relforcerowsecurity)
+ return RLS_ENABLED;
+ else
+ return RLS_NONE_ENV;
+ }

Is the 'else' even necessary in this block?

Other than that, the approach looks good to me.

The patch applied cleanly against master (758fcfd).  As well
'check-world' was successful (obviously understanding that more
related tests are necessary).

> I have a hard time with this.  We're not talking about the application
> logic in this case, we're talking about the guarantees which the
> database is making to the user, be it an application or an individual.
> I've included a patch (the second in the set attached) which adds a
> SECURITY_NOFORCE_RLS bit which has a much tighter scope- it only applies
> after the regular owner check is done.  This reduces the risk of it
> being set mistakenly dramatically, I believe.  Further, the cascaded
> case is correctly handled.  This also needs more polish and regression
> tests, but I wanted to solicit for comment before moving forward with
> that since we don't have a consensus about if this should be done.

I'm not sure that I understand the previous concerns around this bit
well enough to speak intelligently on this point.  However, with that
said, I believe the changes made by this patch make sense.


Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to