On Mon, 14 Mar 2022 at 16:16, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> The patch is fine from my point of view.
>
> It passes "make check-world".
>
> I'll mark it as "ready for committer".
>
Cool, thanks. I think this will make a useful addition to PG15.
I have been hacking on it a bit, and attached is an updated version.
Aside from some general copy editing, the most notable changes are:
In the updatable_views tests, I have moved the new tests to
immediately after the existing permission checking tests, which seems
like a more logical place to put them, and modified them to use the
same style as those existing tests. IMO, this test style makes the
task of writing tests simpler, since the expected output is a little
more obvious.
Similarly in the rowsecurity tests, I have moved the new tests to
immediately after the existing tests for RLS policies on tables
accessed via views, and added a few new tests in the same style,
including verifying permission checks on relations in subqueries in
RLS policies, when the table is accessed via a view.
I wasn't happy with the overall level of test coverage for this new
feature, so I have expanded on them quite a bit. This includes tests
for a bug in rewriteTargetView() -- it wasn't consistently handling
the case of an update involving an ordinary view on top of a security
invoker view.
I have added explicit documentation for the fact that a security
invoker view always does permission checks as the current user, even
if it is accessed from a non-security invoker view, since that was the
cause of some discussion on this thread.
I've also added some more detailed documentation describing how all
this affects RLS, since that's likely to be a common use case.
I've done a fairly extensive doc search, and I *think* I've identified
all the other places that needed updating.
One additional thing that had been missed was that the LOCK command
can be used to lock views, which includes locking all underlying base
relations, after checking permissions as the view owner. The
logical/consistent thing to do for security invoker views is to do the
permission checks as the invoking user, so I've done that.
Barring any other comments or objections, I'll push this in a couple
of days or so, after a bit more proof-reading.
Regards,
Dean