Re: [HACKERS] Broken lock management in policy.c.

* Peter Geoghegan (p...@heroku.com) wrote: > On Mon, Jan 4, 2016 at 12:13 PM, Tom Lane wrote: > > I've pushed an example based on your original test case. Feel free > > to suggest improvements, although at this point they'll probably > > land in 9.5.1. > > I think that

Re: [HACKERS] Broken lock management in policy.c.

[ getting back to this now that there's a little time ] Peter Geoghegan writes: > On Sun, Jan 3, 2016 at 7:01 PM, Peter Geoghegan wrote: >> I would also advise only referencing a single relation within the >> SELECT FOR UPDATE. > To state what may be obvious:

Re: [HACKERS] Broken lock management in policy.c.

On Mon, Jan 4, 2016 at 12:13 PM, Tom Lane wrote: > I've pushed an example based on your original test case. Feel free > to suggest improvements, although at this point they'll probably > land in 9.5.1. I think that that's a vast improvement. I probably should have pushed for

Re: [HACKERS] Broken lock management in policy.c.

I wrote: > I'll go draft something up ... I've pushed an example based on your original test case. Feel free to suggest improvements, although at this point they'll probably land in 9.5.1. regards, tom lane -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 6:09 PM, Peter Geoghegan wrote: >> I think we should get rid of it altogether (since I also agree with the >> upthread comment that it's in the wrong place) and instead put an example >> into section 5.7 saying directly that sub-selects, or in general >>

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 6:00 PM, Stephen Frost wrote: >> I'm not sure what you mean. The CREATE POLICY changes in commit >> 43cd468cf01007f3 specifically call out the issue illustrated in my >> example test case. There are some other changes made in that commit, >> but they

Re: [HACKERS] Broken lock management in policy.c.

Peter Geoghegan writes: > On Sun, Jan 3, 2016 at 5:18 PM, Tom Lane wrote: >> 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.

Re: [HACKERS] Broken lock management in policy.c.

Peter, On Sunday, January 3, 2016, Peter Geoghegan wrote: > On Sun, Jan 3, 2016 at 6:09 PM, Peter Geoghegan > wrote: > >> I think we should get rid of it altogether (since I also agree with the > >> upthread comment that it's in the wrong place)

Re: [HACKERS] Broken lock management in policy.c.

Tom, Peter, On Sunday, January 3, 2016, Peter Geoghegan wrote: > On Sun, Jan 3, 2016 at 4:32 PM, Tom Lane > > wrote: > > CREATE POLICY takes AccessExclusiveLock on the table a policy is being > > added to -- good -- and then releases it when

Re: [HACKERS] Broken lock management in policy.c.

Tom, On Sunday, January 3, 2016, Tom Lane wrote: > CREATE POLICY takes AccessExclusiveLock on the table a policy is being > added to -- good -- and then releases it when done -- bad. Correct > behavior is to hold the lock till commit, because otherwise there is > no

Re: [HACKERS] Broken lock management in policy.c.

Tom, On Sunday, January 3, 2016, Tom Lane wrote: > Peter Geoghegan > writes: > > On Sun, Jan 3, 2016 at 4:32 PM, Tom Lane > wrote: > >> If we fix this, I believe we could also remove the weasel wording that >

Re: [HACKERS] Broken lock management in policy.c.

Peter Geoghegan writes: > On Sun, Jan 3, 2016 at 4:32 PM, Tom Lane 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

Re: [HACKERS] Broken lock management in policy.c.

Stephen Frost writes: > On Sunday, January 3, 2016, Tom Lane wrote: >> CREATE POLICY takes AccessExclusiveLock on the table a policy is being >> added to -- good -- and then releases it when done -- bad. Correct >> behavior is to hold the lock till

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 5:18 PM, Tom Lane wrote: > 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

Re: [HACKERS] Broken lock management in policy.c.

Peter, On Sunday, January 3, 2016, Peter Geoghegan wrote: > On Sun, Jan 3, 2016 at 5:18 PM, Tom Lane > > wrote: > > In any case, the text in create_policy.sgml seems to be a misleading > > description of the problem, as it's talking about DDL

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 7:01 PM, Peter Geoghegan wrote: > I would also advise only referencing a single relation within the > SELECT FOR UPDATE. To state what may be obvious: We should recommend that SELECT FOR SHARE appear in the CREATE POLICY USING qual as part of this

[HACKERS] Broken lock management in policy.c.

CREATE POLICY takes AccessExclusiveLock on the table a policy is being added to -- good -- and then releases it when done -- bad. Correct behavior is to hold the lock till commit, because otherwise there is no guarantee that other backends will see the updated catalog rows in time, or indeed at

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 6:09 PM, Tom Lane wrote: >> Really? But the problem happens as a consequence of having a >> subqueries within CREATE POLICY's USING quals > > If that's what we're talking about, let's say it in precisely that many > words. With an example. The current

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 6:41 PM, Stephen Frost wrote: > A security defined function could be used to address that, of course. That > could have performance implications, naturally. True. I would also advise only referencing a single relation within the SELECT FOR UPDATE. --

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 4:32 PM, Tom Lane wrote: > CREATE POLICY takes AccessExclusiveLock on the table a policy is being > added to -- good -- and then releases it when done -- bad. Correct > behavior is to hold the lock till commit, because otherwise there is > no guarantee

Re: [HACKERS] Broken lock management in policy.c.

On Sun, Jan 3, 2016 at 6:04 PM, Tom Lane wrote: > I'm going to state it as an incontrovertible fact that that paragraph > is so vague as to be useless, because I sure misunderstood it, and in > fact just copy-edited it to talk about changes to RLS policies. I now > see that

Re: [HACKERS] Broken lock management in policy.c.

Peter Geoghegan writes: > On Sun, Jan 3, 2016 at 6:00 PM, Stephen Frost wrote: >> I believe Tom's complaint was that the overall page is about CREATE POLICY, >> technically, and that the text in attempting to address the concern might be >> taken under the