Re: Broken lock management in policy.c.

Tom,

On Sunday, January 3, 2016, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Peter Geoghegan <pg(at)heroku(dot)com <javascript:;>> writes:
> > On Sun, Jan 3, 2016 at 4:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> <javascript:;>> wrote:
> >> If we fix this, I believe we could also remove the weasel wording that
> was
> >> added to create_policy.sgml in commit 43cd468cf01007f3 about how the
> >> system might transiently fail to enforce row security correctly.
>
> > IIUC, then what you say here isn't true, because that issue was about
> > a transient failure without the involvement of *any* DDL from start to
> > finish. CREATE POLICY accepts subqueries referencing other relations
> > in its USING quals. This seems to be idiomatic usage of CREATE POLICY,
> > in fact.
>
> > See my original isolation tester test case, where only the setup
> involves DDL:
>
> >
> http://www.postgresql.org/message-id/attachment/38467/0001-RLS-isolation-test.patch
>
> Hmm. I agree that this test case's behavior does not depend on CREATE
> POLICY's lock mismanagement. I think what is going on here is that the
> RLS quals are being checked with an older snapshot than what controls
> the output of the UPDATE RETURNING. Even the EPQ recheck that's done
> after getting update lock on the "information" row doesn't fix it,
> because we *don't* insist on taking an update lock on the "users" table,
> so we don't see the new value of that row.
>
> If that diagnosis is correct, you could fix it by changing the RLS
> policies' sub-selects to use SELECT FOR UPDATE, though the loss of
> concurrency might well be unacceptable.
>
> In any case, the text in create_policy.sgml seems to be a misleading
> description of the problem, as it's talking about DDL modifications
> which are *not* what's happening here.

There was some debate about the right place for that discussion as there
didn't seem to be any particularly ideal location. I had been intending to
have a locking section in the RLS section rework.

Thanks,

Stephen