Re: Review of Row Level Security

KaiGai, all,

* Kohei KaiGai (kaigai(at)kaigai(dot)gr(dot)jp) wrote:
> 2012/12/21 Kevin Grittner <kgrittn(at)mail(dot)com>:
> > Simon Riggs wrote:
> >
> >> Each table has a single security clause. The clause doesn't enforce
> >> that it must contain something that depends on role, but that is the
> >> most easily understood usage of it. We do that to ensure that you can
> >> embed the intelligence into a function, say hasRowLevelAccess(), which
> >> doesn't have any provable relationship on role, and maybe wouldn't do
> >> anything in unit test, yet clearly in production it would do so.
> >>
> >> It would be easy enough to include read/write variable clauses by
> >> using a function similar to TG_OP for triggers, e.g. StatementType.
> >> That would make the security clause that applied only to reads into
> >> something like this (StatementType('INSERT, UPDATE') OR other-quals).
> >
> > In the case where the logic in encapsulated into a function, what
> > are the logical differences from a BEFORE EACH ROW trigger? If
> > none, and this is strictly an optimization, what are the benchmarks
> > showing?

I'm trying to understand this piece also. It sounds like all we're
doing at the moment is adding different syntax to define a trigger
function and that's hardly what I, at least, was expecting. If a
function has to be written to have RLS, then I think we've failed. Much
of the point of this is to provide an easy solution which gets all the
details right for users who aren't comfortable or savvy enough to just
write functions/security definer views/etc themselves.

> It seems to me we need some more discussion about design and
> implementation on row-security checks of writer-side, to reach our
> consensus.

Again, I agree with Kevin on this- there should be a wiki or similar
which actually outlines the high-level design, syntax, etc. That would
help us understand and be able to meaningfully comment about the
approach.

> On the other hand, we are standing next to the consensus about
> reader-side; a unique row-security policy (so, first version does not
> support per-command policy) shall be checked on table scanning
> on select, update or delete commands.

I don't feel that we've really reached a consensus about the
'reader-side' implemented in this patch- rather, we've agreed (at a
pretty high level) what the default impact of RLS for SELECT queries is.
While I'm glad that we were able to do that, I'm rather dismayed that it
took a great deal of discussion to get to that point.

Thanks,

Stephen