Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-13 Thread Dean Rasheed
On 13 April 2014 05:23, Stephen Frost sfr...@snowman.net wrote: Alright, I've committed this with an updated note regarding the locking, and a few additional regression tests That's awesome. Thank you very much. Regards, Dean -- Sent via pgsql-hackers mailing list

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-12 Thread Stephen Frost
All, * Tom Lane (t...@sss.pgh.pa.us) wrote: Yeah, the point of the gotcha is that a FOR UPDATE specified *outside* a security-barrier view would act as though it had appeared *inside* the view, since it effectively gets pushed down even though outer quals don't. Alright, I've committed this

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-12 Thread Stephen Frost
Greg, * Gregory Smith (gregsmithpg...@gmail.com) wrote: Now that Tom has given some guidance on the row locking weirdness, maybe it's time for me to start updating those into make check form. If you have time, that'd certainly be helpful. The documentation has been in a similar holding

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-11 Thread Dean Rasheed
On 11 April 2014 04:04, Stephen Frost sfr...@snowman.net wrote: Dean, Craig, all, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: This is reflected in the change to the regression test output where, in one of the tests, the ctids for the table to update are no longer coming from the same

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-11 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 11 April 2014 04:04, Stephen Frost sfr...@snowman.net wrote: which changes it to if a relation was changed to a subquery, it had better be because it's got securityQuals that we're dealing with. My general thinking here is that

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-11 Thread Craig Ringer
(Sorry if this breaks the thread history; on mobile) Am I right in thinking that the locking gotcha only happens if you create a security_barrier view conaining a SELECT ... FOR UPDATE? If so, that seems like rather a niche case - not that that means we shouldn't warn people about it.

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-11 Thread Stephen Frost
* Craig Ringer (cr...@2ndquadrant.com) wrote: Hmm, the 'gotcha' I was referring to was the issue discussed upthread around rows getting locked to be updated which didn't pass all the quals (they passed the security_barrier view's, but not the user-supplied ones), which could happen

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-11 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Dean Rasheed (dean.a.rash...@gmail.com) wrote: Am I right in thinking that the locking gotcha only happens if you create a security_barrier view conaining a SELECT ... FOR UPDATE? If so, that seems like rather a niche case - not that that means we

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-10 Thread Gregory Smith
On 4/9/14 9:56 PM, Stephen Frost wrote: As for docs and testing, those are things we would certainly be better off with and may mean that this isn't able to make it into 9.4, which is fair, but I wouldn't toss it out solely due to that. We have a git repo with multiple worked out code

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-10 Thread Stephen Frost
Dean, Craig, all, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: This is reflected in the change to the regression test output where, in one of the tests, the ctids for the table to update are no longer coming from the same table. I think a better approach is to push down the rowmark into

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-09 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 10 March 2014 03:36, Craig Ringer cr...@2ndquadrant.com wrote: I've found an issue with updatable security barrier views. Locking is being pushed down into the subquery. Locking is thus applied before user-supplied quals are, so we potentially

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-09 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: Dean Rasheed dean.a.rash...@gmail.com writes: On 10 March 2014 03:36, Craig Ringer cr...@2ndquadrant.com wrote: I've found an issue with updatable security barrier views. Locking is being pushed down into the subquery. Locking is thus applied before

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-09 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: So if you wanted user-supplied quals to limit which rows get locked physically, they would need to be applied before the lower LockRows node. Right. To my mind, it's not immediately apparent that that is a

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-09 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: That sounds a bit confused. It was- I clearly hadn't followed the thread entirely. The quals aren't getting to see rows they shouldn't be allowed to see. The issue rather is whether rows that the user quals would exclude might get locked anyway

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-04-09 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes: Interesting. I'm trying to reason out why we don't have it already in similar situations; we can't *always* flatten a subquery... I think we do, just nobody's particularly noticed (which further reduces the urgency of finding a solution, I suppose).

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-03-10 Thread Dean Rasheed
On 10 March 2014 03:36, Craig Ringer cr...@2ndquadrant.com wrote: I've found an issue with updatable security barrier views. Locking is being pushed down into the subquery. Locking is thus applied before user-supplied quals are, so we potentially lock too many rows. I'm looking into the code

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-03-09 Thread Craig Ringer
I've found an issue with updatable security barrier views. Locking is being pushed down into the subquery. Locking is thus applied before user-supplied quals are, so we potentially lock too many rows. I'm looking into the code now, but in the mean time, here's a demo of the problem: regress=

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-31 Thread Craig Ringer
On 01/31/2014 05:09 PM, Dean Rasheed wrote: I don't like this fix --- you appear to be adding another RTE to the rangetable (one not in the FROM list) and applying the rowmarks to it, which seems wrong because you're not locking the right set of rows. This is reflected in the change to the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-30 Thread Dean Rasheed
On 30 January 2014 05:36, Craig Ringer cr...@2ndquadrant.com wrote: On 01/29/2014 08:29 PM, Dean Rasheed wrote: On 29 January 2014 11:34, Craig Ringer cr...@2ndquadrant.com wrote: On 01/23/2014 06:06 PM, Dean Rasheed wrote: On 21 January 2014 09:18, Dean Rasheed dean.a.rash...@gmail.com wrote:

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-30 Thread Simon Riggs
On 30 January 2014 11:55, Dean Rasheed dean.a.rash...@gmail.com wrote: Hmm, looks like this is a pre-existing bug. The first thing I tried was to reproduce it using SQL, so I used a RULE to turn a DELETE into a SELECT FOR UPDATE. This is pretty dumb, but it shows the problem: CREATE TABLE

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Craig Ringer
On 01/29/2014 03:34 PM, Kouhei Kaigai wrote: Kouhei Kaigai kai...@ak.jp.nec.com writes: Let me ask an elemental question. What is the reason why inheritance expansion logic should be located on the planner stage, not on the tail of rewriter? I think it's mostly historical. You would however

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Simon Riggs
On 28 January 2014 21:28, Robert Haas robertmh...@gmail.com wrote: On Tue, Jan 28, 2014 at 5:02 AM, Simon Riggs si...@2ndquadrant.com wrote: I agree that this is being seen the wrong way around. The planner contains things it should not do, and as a result we are now discussing enhancing the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Simon Riggs
On 29 January 2014 06:43, Tom Lane t...@sss.pgh.pa.us wrote: Actually though, isn't this issue mostly about inheritance of a query *target* table? Exactly. IMHO updateable views on inheritance sets will have multiple other performance problems, so trying to support them here will not make

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Craig Ringer
On 01/23/2014 06:06 PM, Dean Rasheed wrote: On 21 January 2014 09:18, Dean Rasheed dean.a.rash...@gmail.com wrote: Yes, please review the patch from 09-Jan (http://www.postgresql.org/message-id/CAEZATCUiKxOg=voovja2s6g-sixzzxg18totggp8zobq6qn...@mail.gmail.com). After further testing I

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Dean Rasheed
On 29 January 2014 11:27, Simon Riggs si...@2ndquadrant.com wrote: On 29 January 2014 06:43, Tom Lane t...@sss.pgh.pa.us wrote: Actually though, isn't this issue mostly about inheritance of a query *target* table? Exactly. IMHO updateable views on inheritance sets will have multiple other

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Dean Rasheed
On 29 January 2014 11:34, Craig Ringer cr...@2ndquadrant.com wrote: On 01/23/2014 06:06 PM, Dean Rasheed wrote: On 21 January 2014 09:18, Dean Rasheed dean.a.rash...@gmail.com wrote: Yes, please review the patch from 09-Jan

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Dean Rasheed
On 29 January 2014 06:43, Tom Lane t...@sss.pgh.pa.us wrote: Kouhei Kaigai kai...@ak.jp.nec.com writes: Let me ask an elemental question. What is the reason why inheritance expansion logic should be located on the planner stage, not on the tail of rewriter? I think it's mostly historical.

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-29 Thread Simon Riggs
On 29 January 2014 12:20, Dean Rasheed dean.a.rash...@gmail.com wrote: @Dean: If we moved that to rewriter, would expand_security_quals() and preprocess_rowmarks() also move? Actually I tend to think that expand_security_quals() should remain where it is, regardless of where inheritance

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-28 Thread Simon Riggs
On 28 January 2014 04:10, Kouhei Kaigai kai...@ak.jp.nec.com wrote: AFAICS the only area of objection is the handling of inherited relations, which occurs within the planner in the current patch. I can see that would be a cause for concern since the planner is pluggable and it would then

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-28 Thread Robert Haas
On Tue, Jan 28, 2014 at 5:02 AM, Simon Riggs si...@2ndquadrant.com wrote: I agree that this is being seen the wrong way around. The planner contains things it should not do, and as a result we are now discussing enhancing the code that is in the wrong place, which of course brings objections.

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-28 Thread Tom Lane
Kouhei Kaigai kai...@ak.jp.nec.com writes: Let me ask an elemental question. What is the reason why inheritance expansion logic should be located on the planner stage, not on the tail of rewriter? I think it's mostly historical. You would however have to think of a way to preserve the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-28 Thread Kouhei Kaigai
-Original Message- From: Tom Lane [mailto:t...@sss.pgh.pa.us] Sent: Wednesday, January 29, 2014 3:43 PM To: Kaigai, Kouhei(海外, 浩平) Cc: Craig Ringer; Simon Riggs; Dean Rasheed; PostgreSQL Hackers; Kohei KaiGai; Robert Haas Subject: Re: [HACKERS] WIP patch (v2) for updatable security

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: Am I right in thinking that we have this fully working now? I will look at this at some point during the CF, but have not yet, and probably won't as long as it's not marked ready for committer. regards, tom lane -- Sent via

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Simon Riggs
On 27 January 2014 15:04, Dean Rasheed dean.a.rash...@gmail.com wrote: So for example, when planning the query to update an inheritance child, the rtable will contain an RTE for the parent, but it will not be referenced in the parse tree, and so it will not be expanded while planning the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Dean Rasheed
On 27 January 2014 07:54, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Hello, I checked the latest updatable security barrier view patch. Even though I couldn't find a major design problem in this revision, here are two minor comments below. I think, it needs to be reviewed by committer to

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Simon Riggs
On 27 January 2014 16:11, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: Am I right in thinking that we have this fully working now? I will look at this at some point during the CF, but have not yet, and probably won't as long as it's not marked ready for

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Craig Ringer
On 01/28/2014 12:09 AM, Simon Riggs wrote: On 27 January 2014 15:04, Dean Rasheed dean.a.rash...@gmail.com wrote: So for example, when planning the query to update an inheritance child, the rtable will contain an RTE for the parent, but it will not be referenced in the parse tree, and so it

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-27 Thread Kouhei Kaigai
; Robert Haas Subject: Re: [HACKERS] WIP patch (v2) for updatable security barrier views On 01/28/2014 12:09 AM, Simon Riggs wrote: On 27 January 2014 15:04, Dean Rasheed dean.a.rash...@gmail.com wrote: So for example, when planning the query to update an inheritance child, the rtable

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-26 Thread Kouhei Kaigai
: [HACKERS] WIP patch (v2) for updatable security barrier views On 21 January 2014 09:18, Dean Rasheed dean.a.rash...@gmail.com wrote: Yes, please review the patch from 09-Jan (http://www.postgresql.org/message-id/CAEZATCUiKxOg=vOOvjA2S6G-sixzzxg 18totggp8zobq6qn...@mail.gmail.com). After

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-23 Thread Dean Rasheed
On 23 January 2014 06:13, KaiGai Kohei kai...@ak.jp.nec.com wrote: A minor comment is below: ! /* !* Make sure that the query is marked correctly if the added qual !* has sublinks. !*/ ! if (!parsetree-hasSubLinks) !

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-22 Thread KaiGai Kohei
(2014/01/21 18:18), Dean Rasheed wrote: On 21 January 2014 01:09, KaiGai Kohei kai...@ak.jp.nec.com wrote: (2014/01/13 22:53), Craig Ringer wrote: On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-21 Thread Dean Rasheed
On 21 January 2014 01:09, KaiGai Kohei kai...@ak.jp.nec.com wrote: (2014/01/13 22:53), Craig Ringer wrote: On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-20 Thread KaiGai Kohei
(2014/01/13 22:53), Craig Ringer wrote: On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as other quals are preprocessed. But thinking about

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-20 Thread Craig Ringer
On 01/21/2014 09:09 AM, KaiGai Kohei wrote: (2014/01/13 22:53), Craig Ringer wrote: On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-13 Thread Craig Ringer
On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as other quals are preprocessed. But thinking about it further, those quals are destined

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-13 Thread Dean Rasheed
On 12 January 2014 10:12, Craig Ringer cr...@2ndquadrant.com wrote: On 01/09/2014 06:48 PM, Dean Rasheed wrote: On 8 January 2014 10:19, Dean Rasheed dean.a.rash...@gmail.com wrote: The assertion failure with inheritance and sublinks is a separate issue --- adjust_appendrel_attrs() is not

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-13 Thread Craig Ringer
On 01/09/2014 11:19 PM, Tom Lane wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as other quals are preprocessed. But thinking about it further, those quals are destined

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-12 Thread Craig Ringer
On 01/09/2014 06:48 PM, Dean Rasheed wrote: On 8 January 2014 10:19, Dean Rasheed dean.a.rash...@gmail.com wrote: The assertion failure with inheritance and sublinks is a separate issue --- adjust_appendrel_attrs() is not expecting to find any unplanned sublinks in the query tree when it is

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-09 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as other quals are preprocessed. But thinking about it further, those quals are destined to become the quals of subqueries in the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-09 Thread Dean Rasheed
On 9 January 2014 15:19, Tom Lane t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: My first thought was that it should just preprocess any security barrier quals in subquery_planner() in the same way as other quals are preprocessed. But thinking about it further, those

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-08 Thread Craig Ringer
Dean, Short version - Looks amazing overall. Very clever to zip up the s.b. quals, let the rest of the rewriter and planer do their work normally, then unpack them into subqueries inserted in the planner once inheritance appendrels are expanded, etc. My main concern is that the

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-07 Thread Craig Ringer
I've been thinking about this some more and I don't think you can avoid the need to remap vars and attrs. I agree. I was hoping to let expand_targetlist / preprocess_targetlist do the job, but I'm no longer convinced that'll do the job without mangling them in the process. AIUI, your modified

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2014-01-07 Thread Craig Ringer
On 01/08/2014 02:00 PM, Craig Ringer wrote: I'm not sure I understand how this would work in the face of multiple levels of nesting s.b. views. Are you thinking of doing recursive expansion? Never mind, that part is clearly covered in the patch. -- Craig Ringer

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

2013-12-13 Thread Craig Ringer
Hi all Here's an updated version of the updatable security barrier views patch that's proposed as the next stage of progressing row-security toward useful and maintainable inclusion in core. If updatable security barrier views are available then the row-security code won't have to play around