Re: [HACKERS] RLS open items are vague and unactionable

2015-11-23 Thread Stephen Frost
Noah, * Noah Misch (n...@leadboat.com) wrote: > On Mon, Sep 28, 2015 at 03:03:51PM -0400, Stephen Frost wrote: > > If SELECT rights are required then apply the SELECT policies, even if > > the actual command is an UPDATE or DELETE. This covers the RETURNING > > case which was discussed

Re: [HACKERS] RLS open items are vague and unactionable

2015-11-20 Thread Noah Misch
On Mon, Sep 28, 2015 at 03:03:51PM -0400, Stephen Frost wrote: > If SELECT rights are required then apply the SELECT policies, even if > the actual command is an UPDATE or DELETE. This covers the RETURNING > case which was discussed previously, so we don't need the explicit check > for that, and

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-28 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Sep 15, 2015 at 10:22 AM, Stephen Frost wrote: > > Unless there are other concerns or issues raised, I'll push this later > > today. > > So does this mean that the first RLS open item is addressed? If so, > can it be

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-28 Thread Robert Haas
On Tue, Sep 15, 2015 at 10:22 AM, Stephen Frost wrote: > Dean, > > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> A minor point -- this comment isn't quite right: > > Fixed. > >> because the policies that are fetched there are only used for >> add_security_quals(), not

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-15 Thread Dean Rasheed
On 15 September 2015 at 15:22, Stephen Frost wrote: > Updated patch attached for review. > > Unless there are other concerns or issues raised, I'll push this later > today. > Looks good to me. Thanks for doing all this. Regards, Dean -- Sent via pgsql-hackers mailing list

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-15 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 15 September 2015 at 15:22, Stephen Frost wrote: > > Updated patch attached for review. > > > > Unless there are other concerns or issues raised, I'll push this later > > today. > > Looks good to me. Excellent,

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-15 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > A minor point -- this comment isn't quite right: Fixed. > because the policies that are fetched there are only used for > add_security_quals(), not for add_with_check_options(). It might be > cleaner if the 'if' statement that follows

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-14 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > It shouldn't be necessary to change get_policies_for_relation() at all > to support the RETURNING check. You just need to call it with > CMD_SELECT. Yup, that works well. > BTW, your change to change get_policies_for_relation() has > a

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-14 Thread Dean Rasheed
On 14 September 2015 at 14:47, Stephen Frost wrote: > Attached is a git format-patch built series which includes both commits, > now broken out, for review. > That looks OK to me. A minor point -- this comment isn't quite right: /* * For the target relation, when

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-14 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 14 September 2015 at 14:47, Stephen Frost wrote: > > Attached is a git format-patch built series which includes both commits, > > now broken out, for review. > > That looks OK to me. Excellent. > A minor point --

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-14 Thread Dean Rasheed
On 13 September 2015 at 22:54, Stephen Frost wrote: > Not in front of my laptop and will review it when I get back in more detail, > but the original approach that I tried was changing > get_policies_for_relation to try and build everything necessary, which > didn't work as we

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-13 Thread Dean Rasheed
On 11 September 2015 at 15:51, Joe Conway wrote: > On 09/11/2015 04:33 AM, Stephen Frost wrote: >> * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >>> I will happily work up a rebased version of the patch, but only if I >>> get a serious commitment from a committer to review

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-13 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > OK, here's a rebased version of the patch. Thanks! > There are no significant changes from last time this was discussed. I > believe the module regression test changes are harmless --- a result > of a change in the order that SB quals are

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-13 Thread Stephen Frost
Dean, On Sunday, September 13, 2015, Dean Rasheed wrote: > On 13 September 2015 at 20:59, Stephen Frost > wrote: > > I've been through this and definitely like it more than what we had > > previously, as I had mentioned previously.

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-13 Thread Dean Rasheed
On 13 September 2015 at 20:59, Stephen Frost wrote: > I've been through this and definitely like it more than what we had > previously, as I had mentioned previously. I've also added the > RETURNING handling, per the discussion between you, Robert, Tom, and > Kevin. > > Would

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Tom Lane
Dean Rasheed writes: > On 11 September 2015 at 15:49, Tom Lane wrote: >> However, I don't see why UPDATE/DELETE with RETURNING couldn't be >> restricted according to *both* the UPDATE and SELECT policies, >> ie if there's RETURNING then you can't

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Dean Rasheed
On 11 September 2015 at 17:56, Tom Lane wrote: > Dean Rasheed writes: >> On 11 September 2015 at 15:49, Tom Lane wrote: >>> However, I don't see why UPDATE/DELETE with RETURNING couldn't be >>> restricted according to *both* the

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Dean Rasheed
On 11 September 2015 at 15:49, Tom Lane wrote: > Dean Rasheed writes: >> Yeah, we had a similar discussion regarding UPDATE USING policies and >> ON CONFLICT UPDATE clauses. I think the argument against filtering is >> that the rows returned would

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Dean Rasheed
On 10 September 2015 at 21:48, Robert Haas wrote: > The open items list here: > > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items > > Dean's latest round of RLS refactoring. This refactoring patch doesn't fix any behavioural issues. It is all about trying to

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Kevin Grittner
Dean Rasheed wrote: > Ultimately I think this will be an extremely rare case, probably more > likely to happen as a result of accidentally misconfigured policies. > But if that does happen, I'd rather have an error to alert me to the > fact, than to silently do nothing.

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Stephen Frost
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Sep 11, 2015 at 7:33 AM, Stephen Frost wrote: > > I've updated the page to add more details about the various items, > > though the only code changes at this point considered 'open' are this > > refactoring and

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Dean Rasheed
On 11 September 2015 at 13:05, Robert Haas wrote: > Thanks for the updates. My thoughts: > > On RETURNING, it seems like we've got a fairly fundamental problem > here. If I understand correctly, the intention of allowing policies > to be filtered by command type is to

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Robert Haas
On Fri, Sep 11, 2015 at 7:33 AM, Stephen Frost wrote: > I've updated the page to add more details about the various items, > though the only code changes at this point considered 'open' are this > refactoring and the question regarding the 'row-level-security disabled' >

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Stephen Frost
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 10 September 2015 at 21:48, Robert Haas wrote: > > The open items list here: > > > > https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items > > > > Dean's latest round of RLS refactoring. > > This refactoring

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > I think blind updates are a pretty niche case, and I think that it > wasn't the intention to support them, but more of an unintentional > side effect of the implementation. That said, there are > just-about-plausible use-cases where they might be

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Tom Lane
Dean Rasheed writes: > Yeah, we had a similar discussion regarding UPDATE USING policies and > ON CONFLICT UPDATE clauses. I think the argument against filtering is > that the rows returned would then be misleading about what was > actually updated. It seems to me that

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Dean Rasheed
On 11 September 2015 at 15:20, Stephen Frost wrote: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> I think blind updates are a pretty niche case, and I think that it >> wasn't the intention to support them, but more of an unintentional >> side effect of the

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Joe Conway
On 09/11/2015 04:33 AM, Stephen Frost wrote: > * Dean Rasheed (dean.a.rash...@gmail.com) wrote: >> I will happily work up a rebased version of the patch, but only if I >> get a serious commitment from a committer to review it. Otherwise, >> I'll let it drop. > > There's no question about getting

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Robert Haas
On Fri, Sep 11, 2015 at 10:49 AM, Tom Lane wrote: > Dean Rasheed writes: >> Yeah, we had a similar discussion regarding UPDATE USING policies and >> ON CONFLICT UPDATE clauses. I think the argument against filtering is >> that the rows returned would

Re: [HACKERS] RLS open items are vague and unactionable

2015-09-11 Thread Robert Haas
On Fri, Sep 11, 2015 at 9:48 AM, Stephen Frost wrote: > The only reason to avoid providing that flexibility is the concern that > it might be misunderstood and users might misconfigure their system. > Removing the flexibility to have per-command visibility policies and >