Re: [HACKERS] Improving RLS planning

Andreas Seltenreich writes: > sqlsmith doesn't seem to like 215b43cdc: > select 1 from information_schema.usage_privileges > where information_schema._pg_keysequal( >(select null::smallint[]), >'{16,25,23}'); > -- TRAP: FailedAssertion("!(!and_clause((Node *)

Re: [HACKERS] Improving RLS planning

Tom Lane writes: > Thanks for reviewing --- I'll do something with that test case and > push it. sqlsmith doesn't seem to like 215b43cdc: select 1 from information_schema.usage_privileges where information_schema._pg_keysequal( (select null::smallint[]), '{16,25,23}'); -- TRAP:

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Thanks for the review. Attached is an updated patch that I believe >> addresses all of the review comments so far. The code is unchanged from >> v2, but I improved the README, some comments, and the regression

Re: [HACKERS] Improving RLS planning

Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> Here's an updated version of the RLS planning patch that gets rid of > >> the incorrect interaction with Query.hasRowSecurity and adjusts > >> terminology as

Re: [HACKERS] Improving RLS planning

On 29 December 2016 at 15:55, Tom Lane wrote: > Dean Rasheed writes: >> On 28 December 2016 at 19:12, Tom Lane wrote: >>> [ getting back to this patch finally... ] I made the suggested change >>> to that test case, and what I

Re: [HACKERS] Improving RLS planning

Dean Rasheed writes: > On 28 December 2016 at 19:12, Tom Lane wrote: >> [ getting back to this patch finally... ] I made the suggested change >> to that test case, and what I see is a whole lot of "NOTICE: snooped value >> = whatever" outputs. >>

Re: [HACKERS] Improving RLS planning

On 28 December 2016 at 19:12, Tom Lane wrote: > Stephen Frost writes: >> * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >>> Hmm. I've not read any of the new code yet, but the fact that this >>> test now reduces to a one-time filter makes it effectively

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Here's an updated version of the RLS planning patch that gets rid of >> the incorrect interaction with Query.hasRowSecurity and adjusts >> terminology as agreed. > I've spent a fair bit of time going over this

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> Hmm. I've not read any of the new code yet, but the fact that this >> test now reduces to a one-time filter makes it effectively useless as >> a test of qual evaluation order because it has deduced

Re: [HACKERS] Improving RLS planning

Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > Hmm. I've not read any of the new code yet, but the fact that this > test now reduces to a one-time filter makes it effectively useless as > a test of qual evaluation order because it has deduced that it doesn't > need to evaluate them. I

Re: [HACKERS] Improving RLS planning

On 28 November 2016 at 23:55, Stephen Frost wrote: >> diff --git a/src/test/regress/expected/updatable_views.out >> b/src/test/regress/expected/updatable_views.out > [...] >> --- 2104,2114 >> >> EXPLAIN (VERBOSE, COSTS OFF) >> UPDATE v1 SET a=100 WHERE snoop(a) AND

Re: [HACKERS] Improving RLS planning

Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Nov 28, 2016 at 6:55 PM, Stephen Frost wrote: > >> diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README > >> [...] > >> + Additional rules will be needed to support safe handling of join quals

Re: [HACKERS] Improving RLS planning

Robert Haas writes: > On Mon, Nov 28, 2016 at 6:55 PM, Stephen Frost wrote: >> Are you thinking that we will be able to remove the cut-out in >> is_simple_subquery() which currently punts whenever an RTE is marked as >> security_barrier? That would

Re: [HACKERS] Improving RLS planning

On Mon, Nov 28, 2016 at 6:55 PM, Stephen Frost wrote: >> diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README >> [...] >> + Additional rules will be needed to support safe handling of join quals >> + when there is a mix of security levels among join quals;

Re: [HACKERS] Improving RLS planning

Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > >> +1 for that terminology and no renaming of fields. > > > Agreed. > > Here's an updated version of the RLS planning patch that gets rid of > the

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> +1 for that terminology and no renaming of fields. > Agreed. Here's an updated version of the RLS planning patch that gets rid of the incorrect interaction with Query.hasRowSecurity and adjusts

Re: [HACKERS] Improving RLS planning

* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 10 November 2016 at 17:12, Tom Lane wrote: > > Yeah, I think we'd be best off to avoid the bare term "security". > > It's probably too late to change the RTE field name "securityQuals", > > but maybe we could uniformly call

Re: [HACKERS] Improving RLS planning

On 10 November 2016 at 17:12, Tom Lane wrote: > Yeah, I think we'd be best off to avoid the bare term "security". > It's probably too late to change the RTE field name "securityQuals", > but maybe we could uniformly call those "security barrier quals" in > the comments. Then

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> On 8 November 2016 at 14:45, Tom Lane wrote: >>> ... I'm still suspicious that the three places I found may >>> represent bugs in the management of Query.hasRowSecurity. >> I

Re: [HACKERS] Improving RLS planning

Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 8 November 2016 at 14:45, Tom Lane wrote: > > OK. In that case I'll need to adjust the patch so that the planner keeps > > its own flag about whether the query contains any securityQuals; that's > > easy enough.

Re: [HACKERS] Improving RLS planning

On 8 November 2016 at 16:46, Robert Haas wrote: > On Thu, Nov 3, 2016 at 6:23 PM, Tom Lane wrote: >>> I think that ordering might be sub-optimal if you had a mix of >>> leakproof quals and security quals and the cost of some security quals >>> were

Re: [HACKERS] Improving RLS planning

On 8 November 2016 at 14:45, Tom Lane wrote: > Stephen Frost writes: >> * Tom Lane (t...@sss.pgh.pa.us) wrote: >>> * Since the planner is now depending on Query.hasRowSecurity to be set >>> whenever there are any securityQuals, I put in an Assert about

Re: [HACKERS] Improving RLS planning

On Thu, Nov 3, 2016 at 6:23 PM, Tom Lane wrote: >> I think that ordering might be sub-optimal if you had a mix of >> leakproof quals and security quals and the cost of some security quals >> were significantly higher than the cost of some other quals. Perhaps >> all leakproof

Re: [HACKERS] Improving RLS planning

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> * Since the planner is now depending on Query.hasRowSecurity to be set >> whenever there are any securityQuals, I put in an Assert about that, >> and promptly found three places in prepjointree.c and the

Re: [HACKERS] Improving RLS planning

Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Dean Rasheed writes: > > On 25 October 2016 at 22:58, Tom Lane wrote: > >> The alternative I'm now thinking about pursuing is to get rid of the > >> conversion of RLS quals to subqueries. Instead, we

Re: [HACKERS] Improving RLS planning

Dean Rasheed writes: > On 25 October 2016 at 22:58, Tom Lane wrote: >> The alternative I'm now thinking about pursuing is to get rid of the >> conversion of RLS quals to subqueries. Instead, we can label individual >> qual clauses with security

Re: [HACKERS] Improving RLS planning

On 25 October 2016 at 22:58, Tom Lane wrote: > The alternative I'm now thinking about pursuing is to get rid of the > conversion of RLS quals to subqueries. Instead, we can label individual > qual clauses with security precedence markings. Concretely, suppose we > add an

Re: [HACKERS] Improving RLS planning

Robert Haas writes: > On Wed, Oct 26, 2016 at 1:20 PM, Tom Lane wrote: >> Right, so quals from above the SB view would have to not be allowed to >> drop below the join level (but they could fall *to* the join level, >> where they'd be applied after the

Re: [HACKERS] Improving RLS planning

On Wed, Oct 26, 2016 at 1:20 PM, Tom Lane wrote: > Robert Haas writes: >> This might work for RLS policies, if they can only reference a single >> table, but I can't see how it's going to work for security barrier >> views. For example, consider CREATE

Re: [HACKERS] Improving RLS planning

Robert Haas writes: > This might work for RLS policies, if they can only reference a single > table, but I can't see how it's going to work for security barrier > views. For example, consider CREATE VIEW v WITH (security_barrier) AS > SELECT * FROM x, y WHERE x.a = y.a

Re: [HACKERS] Improving RLS planning

On Tue, Oct 25, 2016 at 5:58 PM, Tom Lane wrote: > The alternative I'm now thinking about pursuing is to get rid of the > conversion of RLS quals to subqueries. Instead, we can label individual > qual clauses with security precedence markings. Concretely, suppose we > add an

Re: [HACKERS] Improving RLS planning

On 26 October 2016 at 10:58, Tom Lane wrote: > The alternative I'm now thinking about pursuing is to get rid of the > conversion of RLS quals to subqueries. Instead, we can label individual > qual clauses with security precedence markings. Concretely, suppose we > add an