* Stephen Frost (sfr...@snowman.net) wrote:
> Updated patch attached.
Erp, actually attached this time.
Thanks again!
Stephen
From 27e5fdac801cecc5ac33daccf979bbc59458dbbc Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Thu, 1 Sep 2016 02:11:30 -0400
Subject: [PATCH] Add support for restrict
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> This note reads a little awkwardly to me. I think I would write it as:
>
> Note that ALTER POLICY only allows the set of roles
> to which the policy applies and the USING and
> WITH CHECK expressions to be modified. To change
>
Stephen,
I looked through this in a little more detail and I found one other
issue: the documentation for the system catalog table pg_policy and
the view pg_policies needs to be updated to include the new columns
that have been added.
Other than that, it all looks good to me, subject to the previ
On Fri, Dec 2, 2016 at 2:09 AM, Stephen Frost wrote:
> Dean,
>
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> > On 1 December 2016 at 14:38, Stephen Frost wrote:
> > > * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> > >> In get_policies_for_relation() ...
> > >> ... I think it should so
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 1 December 2016 at 14:38, Stephen Frost wrote:
> > * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> >> In get_policies_for_relation() ...
> >> ... I think it should sort the restrictive policies by name
> >
> > Hmmm, is it really the c
On 1 December 2016 at 14:38, Stephen Frost wrote:
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> In get_policies_for_relation() ...
>> ... I think it should sort the restrictive policies by name
>
> Hmmm, is it really the case that the quals will always end up being
> evaluated in that orde
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 30 November 2016 at 21:54, Stephen Frost wrote:
> > Unless there's further comments, I'll plan to push this sometime
> > tomorrow.
>
> Sorry I didn't have time to look at this properly. I was intending to,
> but my day job just keeps ge
On 30 November 2016 at 21:54, Stephen Frost wrote:
> Unless there's further comments, I'll plan to push this sometime
> tomorrow.
>
Sorry I didn't have time to look at this properly. I was intending to,
but my day job just keeps getting in the way. I do have a couple of
comments relating to the d
* Stephen Frost (sfr...@snowman.net) wrote:
> * Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
> > 4. It will be good if we have an example for this in section
> > "5.7. Row Security Policies"
>
> I haven't added one yet, but will plan to do so.
I've now added and cleaned up the Row Securi
On Thu, Sep 29, 2016 at 7:18 PM, Jeevan Chalke
wrote:
> Hi Stephen,
>
>
>> > 4. It will be good if we have an example for this in section
>> > "5.7. Row Security Policies"
>>
>> I haven't added one yet, but will plan to do so.
>>
> I think you are going to add this in this patch itself, right?
>
>
Hi Stephen,
> 4. It will be good if we have an example for this in section
> > "5.7. Row Security Policies"
>
> I haven't added one yet, but will plan to do so.
>
> I think you are going to add this in this patch itself, right?
I have reviewed your latest patch and it fixes almost all my review
On 27 September 2016 at 15:15, Jeevan Chalke
wrote:
> Hello Stephen,
>
> On Tue, Sep 27, 2016 at 12:57 AM, Stephen Frost wrote:
>>
>> Jeevan,
>>
>> * Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
>> > I have started reviewing this patch and here are couple of points I have
>> > observed s
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen, the typo "awseome" in the tests is a bit distracting. Can you
> please fix it?
Done.
> I think you should use braces here, not parens:
Fixed.
> I don't think this paragraph is right -- you should call out each of the
> values PERMIS
Jeevan,
* Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
> Here are the review comments:
[... many good comments ...]
Many thanks for the review, I believe I agree with pretty much all your
comments and will update the patch accordingly.
Responses for a couple of them are below.
> 6. I
On Tue, Sep 27, 2016 at 12:45 PM, Jeevan Chalke <
jeevan.cha...@enterprisedb.com> wrote:
> Hello Stephen,
>
> I am reviewing the latest patch in detail now and will post my review
> comments later.
>
Here are the review comments:
1. In documentation, we should put both permissive as well as r
Hello Stephen,
On Tue, Sep 27, 2016 at 12:57 AM, Stephen Frost wrote:
> Jeevan,
>
> * Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
> > I have started reviewing this patch and here are couple of points I have
> > observed so far:
> >
> > 1. Patch applies cleanly
> > 2. make / make instal
Stephen Frost wrote:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Stephen Frost wrote:
> > > +
> > > + If the policy is a "permissive" or "restrictive" policy.
> > > Permissive
> > > + policies are the default and always add visibillity- they ar "OR"d
> > > + tog
Alvaro,
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
>
> Stephen, the typo "awseome" in the tests is a bit distracting. Can you
> please fix it?
Will fix.
> > Updated patch changes to using IDENT and an strcmp() (similar to
> > AlterOptRoleElem and vacuum_option_el
Stephen Frost wrote:
Stephen, the typo "awseome" in the tests is a bit distracting. Can you
please fix it?
> Updated patch changes to using IDENT and an strcmp() (similar to
> AlterOptRoleElem and vacuum_option_elem) to check the results at parse-time,
> and then throwing a more specific error i
Jeevan, all,
* Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
> On Mon, Sep 12, 2016 at 7:27 AM, Stephen Frost wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> > > On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane wrote:
> > > > Stephen Frost writes:
> > > >> * Alvaro Herrera (alvhe...@2
Jeevan,
* Jeevan Chalke (jeevan.cha...@enterprisedb.com) wrote:
> I have started reviewing this patch and here are couple of points I have
> observed so far:
>
> 1. Patch applies cleanly
> 2. make / make install / initdb all good.
> 3. make check (regression) FAILED. (Attached diff file for refer
Hi,
I have started reviewing this patch and here are couple of points I have
observed so far:
1. Patch applies cleanly
2. make / make install / initdb all good.
3. make check (regression) FAILED. (Attached diff file for reference).
Please have a look over failures.
Meanwhile I will go ahead and
On Mon, Sep 12, 2016 at 7:27 AM, Stephen Frost wrote:
> Robert,
>
> * Robert Haas (robertmh...@gmail.com) wrote:
> > On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane wrote:
> > > Stephen Frost writes:
> > >> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > >>> Can't you keep those words as Sconst
Robert,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane wrote:
> > Stephen Frost writes:
> >> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >>> Can't you keep those words as Sconst or something (DefElems?) until the
> >>> execution phase, so that th
On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane wrote:
> Stephen Frost writes:
>> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>>> Can't you keep those words as Sconst or something (DefElems?) until the
>>> execution phase, so that they don't need to be keywords at all?
>
>> Seems like we could do
Stephen Frost writes:
> I saw the various list-based cases and commented in my reply (the one you
> quote part of above) why those didn't seem to make sense for this case
> (it's not a list and I don't see it ever growing into one).
I think Alvaro was simply questioning whether there would ever b
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost writes:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >> Can't you keep those words as Sconst or something (DefElems?) until the
> >> execution phase, so that they don't need to be keywords at all?
>
> > Seems like we could do that
Stephen Frost writes:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>> Can't you keep those words as Sconst or something (DefElems?) until the
>> execution phase, so that they don't need to be keywords at all?
> Seems like we could do that, though I'm not convinced that it really
> gains u
Alvaro,
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > * Stephen Frost (sfr...@snowman.net) wrote:
> > > Based on Robert's suggestion and using Thom's verbiage, I've tested this
> > > out:
> > >
> > > CREATE POLICY pol ON tab AS [PERMISSIVE|RESTRICTIVE] ...
>
> Can
Stephen Frost wrote:
> Greetings!
>
> * Stephen Frost (sfr...@snowman.net) wrote:
> > Based on Robert's suggestion and using Thom's verbiage, I've tested this
> > out:
> >
> > CREATE POLICY pol ON tab AS [PERMISSIVE|RESTRICTIVE] ...
Can't you keep those words as Sconst or something (DefElems?) u
Greetings!
* Stephen Frost (sfr...@snowman.net) wrote:
> Based on Robert's suggestion and using Thom's verbiage, I've tested this
> out:
>
> CREATE POLICY pol ON tab AS [PERMISSIVE|RESTRICTIVE] ...
>
> and it appears to work fine with the grammar, etc.
>
> Unless there's other thoughts on this,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Thu, Sep 1, 2016 at 12:04 PM, Stephen Frost wrote:
> > As outlined in the commit message, this adds support for restrictive RLS
> > policies. We've had this in the backend since 9.5, but they were only
> > available via hooks and therefore extensi
On 1 September 2016 at 10:02, Robert Haas wrote:
> On Thu, Sep 1, 2016 at 12:04 PM, Stephen Frost wrote:
>> As outlined in the commit message, this adds support for restrictive RLS
>> policies. We've had this in the backend since 9.5, but they were only
>> available via hooks and therefore exten
On Thu, Sep 1, 2016 at 12:04 PM, Stephen Frost wrote:
> As outlined in the commit message, this adds support for restrictive RLS
> policies. We've had this in the backend since 9.5, but they were only
> available via hooks and therefore extensions. This adds support for
> them to be configured t
34 matches
Mail list logo
