> 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
--
Adam Brightwell - adam.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
