Обсуждение: Odd behavior of updatable security barrier views on foreign tables

Поиск
Список
Период
Сортировка

Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
Hi,

I noticed that when updating security barrier views on foreign tables,
we fail to give FOR UPDATE to selection queries issued at ForeignScan.
Here is an example.

postgres=# create foreign table base_ftbl (person text, visibility text)
server loopback options (table_name 'base_tbl');
CREATE FOREIGN TABLE
postgres=# create view rw_view as select person from base_ftbl where
visibility = 'public';
CREATE VIEW
postgres=# explain verbose delete from rw_view;                                             QUERY PLAN
-------------------------------------------------------------------------------------------------------Delete on
public.base_ftbl (cost=100.00..144.40 rows=14 width=6)  Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1  ->
ForeignScan on public.base_ftbl  (cost=100.00..144.40 rows=14
 
width=6)        Output: base_ftbl.ctid        Remote SQL: SELECT ctid FROM public.base_tbl WHERE ((visibility
= 'public'::text)) FOR UPDATE
(5 rows)

postgres=# alter view rw_view set (security_barrier = true);
ALTER VIEW
postgres=# explain verbose delete from rw_view;                                           QUERY PLAN
--------------------------------------------------------------------------------------------------Delete on
public.base_ftblbase_ftbl_1  (cost=100.00..144.54 rows=14
 
width=6)  Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1  ->  Subquery Scan on base_ftbl  (cost=100.00..144.54
rows=14width=6)        Output: base_ftbl.ctid        ->  Foreign Scan on public.base_ftbl base_ftbl_2
 
(cost=100.00..144.40 rows=14 width=6)              Output: base_ftbl_2.ctid              Remote SQL: SELECT ctid FROM
public.base_tblWHERE
 
((visibility = 'public'::text))
(7 rows)

Correct me if I am wrong.

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Robert Haas
Дата:
On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> I noticed that when updating security barrier views on foreign tables,
> we fail to give FOR UPDATE to selection queries issued at ForeignScan.
> Here is an example.
>
> postgres=# create foreign table base_ftbl (person text, visibility text)
> server loopback options (table_name 'base_tbl');
> CREATE FOREIGN TABLE
> postgres=# create view rw_view as select person from base_ftbl where
> visibility = 'public';
> CREATE VIEW
> postgres=# explain verbose delete from rw_view;
>                                               QUERY PLAN
> -------------------------------------------------------------------------------------------------------
>  Delete on public.base_ftbl  (cost=100.00..144.40 rows=14 width=6)
>    Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
>    ->  Foreign Scan on public.base_ftbl  (cost=100.00..144.40 rows=14
> width=6)
>          Output: base_ftbl.ctid
>          Remote SQL: SELECT ctid FROM public.base_tbl WHERE ((visibility
> = 'public'::text)) FOR UPDATE
> (5 rows)
>
> postgres=# alter view rw_view set (security_barrier = true);
> ALTER VIEW
> postgres=# explain verbose delete from rw_view;
>                                             QUERY PLAN
> --------------------------------------------------------------------------------------------------
>  Delete on public.base_ftbl base_ftbl_1  (cost=100.00..144.54 rows=14
> width=6)
>    Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
>    ->  Subquery Scan on base_ftbl  (cost=100.00..144.54 rows=14 width=6)
>          Output: base_ftbl.ctid
>          ->  Foreign Scan on public.base_ftbl base_ftbl_2
> (cost=100.00..144.40 rows=14 width=6)
>                Output: base_ftbl_2.ctid
>                Remote SQL: SELECT ctid FROM public.base_tbl WHERE
> ((visibility = 'public'::text))
> (7 rows)
>
> Correct me if I am wrong.

That looks like a bug to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
> > I noticed that when updating security barrier views on foreign tables,
> > we fail to give FOR UPDATE to selection queries issued at ForeignScan.
> > Here is an example.
[...]
> > postgres=# alter view rw_view set (security_barrier = true);
> > ALTER VIEW
> > postgres=# explain verbose delete from rw_view;
> >                                             QUERY PLAN
> > --------------------------------------------------------------------------------------------------
> >  Delete on public.base_ftbl base_ftbl_1  (cost=100.00..144.54 rows=14
> > width=6)
> >    Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
> >    ->  Subquery Scan on base_ftbl  (cost=100.00..144.54 rows=14 width=6)
> >          Output: base_ftbl.ctid
> >          ->  Foreign Scan on public.base_ftbl base_ftbl_2
> > (cost=100.00..144.40 rows=14 width=6)
> >                Output: base_ftbl_2.ctid
> >                Remote SQL: SELECT ctid FROM public.base_tbl WHERE
> > ((visibility = 'public'::text))
> > (7 rows)
> >
> > Correct me if I am wrong.
>
> That looks like a bug to me.

Agreed.  I've been looking at this and I suspect it's related to the
discussion around prepsecurity.c and generating the security barrier
subquery that I've been having with Dean.  An initial look, at least,
shows that GetForeignPlan is looking at the subquery instead of the base
relation (as it expects to be).

I'll continue digging into it.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
* Stephen Frost (sfrost@snowman.net) wrote:
> * Robert Haas (robertmhaas@gmail.com) wrote:
> > On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
> > <fujita.etsuro@lab.ntt.co.jp> wrote:
> > > I noticed that when updating security barrier views on foreign tables,
> > > we fail to give FOR UPDATE to selection queries issued at ForeignScan.
> > > Here is an example.
> [...]
> > > postgres=# alter view rw_view set (security_barrier = true);
> > > ALTER VIEW
> > > postgres=# explain verbose delete from rw_view;
> > >                                             QUERY PLAN
> > > --------------------------------------------------------------------------------------------------
> > >  Delete on public.base_ftbl base_ftbl_1  (cost=100.00..144.54 rows=14
> > > width=6)
> > >    Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
> > >    ->  Subquery Scan on base_ftbl  (cost=100.00..144.54 rows=14 width=6)
> > >          Output: base_ftbl.ctid
> > >          ->  Foreign Scan on public.base_ftbl base_ftbl_2
> > > (cost=100.00..144.40 rows=14 width=6)
> > >                Output: base_ftbl_2.ctid
> > >                Remote SQL: SELECT ctid FROM public.base_tbl WHERE
> > > ((visibility = 'public'::text))
> > > (7 rows)
> > >
> > > Correct me if I am wrong.
> >
> > That looks like a bug to me.
>
> Agreed.  I've been looking at this and I suspect it's related to the
> discussion around prepsecurity.c and generating the security barrier
> subquery that I've been having with Dean.  An initial look, at least,
> shows that GetForeignPlan is looking at the subquery instead of the base
> relation (as it expects to be).
>
> I'll continue digging into it.

I've looked into this a fair bit more over the weekend and the issue
appears to be that the FDW isn't expecting a do-instead sub-query.
I've been considering how we might be able to address that but havn't
come up with any particularly great ideas and would welcome any
suggestions.  Simply having the FDW try to go up through the query would
likely end up with too many queries showing up with 'for update'.  We
add the 'for update' to the sub-query before we even get called from
the 'Modify' path too, which means we can't use that to realize when
we're getting ready to modify rows and therefore need to lock them.

In any case, I'll continue to look but would welcome any other thoughts.
Thanks,
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Dean Rasheed
Дата:
On 9 February 2015 at 21:17, Stephen Frost <sfrost@snowman.net> wrote:
>> > On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
>> > > I noticed that when updating security barrier views on foreign tables,
>> > > we fail to give FOR UPDATE to selection queries issued at ForeignScan.
>>
> I've looked into this a fair bit more over the weekend and the issue
> appears to be that the FDW isn't expecting a do-instead sub-query.
> I've been considering how we might be able to address that but havn't
> come up with any particularly great ideas and would welcome any
> suggestions.  Simply having the FDW try to go up through the query would
> likely end up with too many queries showing up with 'for update'.  We
> add the 'for update' to the sub-query before we even get called from
> the 'Modify' path too, which means we can't use that to realize when
> we're getting ready to modify rows and therefore need to lock them.
>
> In any case, I'll continue to look but would welcome any other thoughts.
>

Sorry, I didn't have time to look at this properly. My initial thought
is that expand_security_qual() needs to request a lock on rows coming
from the relation it pushes down into a subquery if that relation was
the result relation, because otherwise it won't have any locks, since
preprocess_rowmarks() only adds PlanRowMarks to non-target relations.

Of course that means that it may end up locking more rows than are
actually updated, but that's essentially the same as a SELECT FOR
UPDATE on a s.b. view right now.

Regards,
Dean



Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/02/10 7:23, Dean Rasheed wrote:
> On 9 February 2015 at 21:17, Stephen Frost <sfrost@snowman.net> wrote:
>>>> On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
>>>>> I noticed that when updating security barrier views on foreign tables,
>>>>> we fail to give FOR UPDATE to selection queries issued at ForeignScan.
>>>
>> I've looked into this a fair bit more over the weekend and the issue
>> appears to be that the FDW isn't expecting a do-instead sub-query.
>> I've been considering how we might be able to address that but havn't
>> come up with any particularly great ideas and would welcome any
>> suggestions.  Simply having the FDW try to go up through the query would
>> likely end up with too many queries showing up with 'for update'.  We
>> add the 'for update' to the sub-query before we even get called from
>> the 'Modify' path too, which means we can't use that to realize when
>> we're getting ready to modify rows and therefore need to lock them.
>>
>> In any case, I'll continue to look but would welcome any other thoughts.

> Sorry, I didn't have time to look at this properly. My initial thought
> is that expand_security_qual() needs to request a lock on rows coming
> from the relation it pushes down into a subquery if that relation was
> the result relation, because otherwise it won't have any locks, since
> preprocess_rowmarks() only adds PlanRowMarks to non-target relations.

That seems close to what I had in mind; expand_security_qual() needs to 
request a FOR UPDATE lock on rows coming from the relation it pushes 
down into a subquery only when that relation is the result relation and 
*foreign table*.

Thanks for dicussing this issue!

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Dean,

* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 9 February 2015 at 21:17, Stephen Frost <sfrost@snowman.net> wrote:
> >> > On Fri, Jan 30, 2015 at 5:20 AM, Etsuro Fujita
> >> > > I noticed that when updating security barrier views on foreign tables,
> >> > > we fail to give FOR UPDATE to selection queries issued at ForeignScan.
> >>
> > I've looked into this a fair bit more over the weekend and the issue
> > appears to be that the FDW isn't expecting a do-instead sub-query.
> > I've been considering how we might be able to address that but havn't
> > come up with any particularly great ideas and would welcome any
> > suggestions.  Simply having the FDW try to go up through the query would
> > likely end up with too many queries showing up with 'for update'.  We
> > add the 'for update' to the sub-query before we even get called from
> > the 'Modify' path too, which means we can't use that to realize when
> > we're getting ready to modify rows and therefore need to lock them.
> >
> > In any case, I'll continue to look but would welcome any other thoughts.
>
> Sorry, I didn't have time to look at this properly. My initial thought
> is that expand_security_qual() needs to request a lock on rows coming
> from the relation it pushes down into a subquery if that relation was
> the result relation, because otherwise it won't have any locks, since
> preprocess_rowmarks() only adds PlanRowMarks to non-target relations.

Yes, that works.  I had been focused on trying to figure out a way to
make this work just in the FDW, but you're right, fixing it in
expand_security_qual() looks like the right approach.

> Of course that means that it may end up locking more rows than are
> actually updated, but that's essentially the same as a SELECT FOR
> UPDATE on a s.b. view right now.

Agreed.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Etsuro,

* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> On 2015/02/10 7:23, Dean Rasheed wrote:
> >Sorry, I didn't have time to look at this properly. My initial thought
> >is that expand_security_qual() needs to request a lock on rows coming
> >from the relation it pushes down into a subquery if that relation was
> >the result relation, because otherwise it won't have any locks, since
> >preprocess_rowmarks() only adds PlanRowMarks to non-target relations.
>
> That seems close to what I had in mind; expand_security_qual() needs
> to request a FOR UPDATE lock on rows coming from the relation it
> pushes down into a subquery only when that relation is the result
> relation and *foreign table*.

I had been trying to work out an FDW-specific way to address this, but I
think Dean's right that this should be addressed in
expand_security_qual(), which means it'll apply to all cases and not
just these FDW calls.  I don't think that's actually an issue though and
it'll match up to how SELECT FOR UPDATE is handled today.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/02/11 4:06, Stephen Frost wrote:
> * Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
>> On 2015/02/10 7:23, Dean Rasheed wrote:
>>> Sorry, I didn't have time to look at this properly. My initial thought
>>> is that expand_security_qual() needs to request a lock on rows coming
>> >from the relation it pushes down into a subquery if that relation was
>>> the result relation, because otherwise it won't have any locks, since
>>> preprocess_rowmarks() only adds PlanRowMarks to non-target relations.
>>
>> That seems close to what I had in mind; expand_security_qual() needs
>> to request a FOR UPDATE lock on rows coming from the relation it
>> pushes down into a subquery only when that relation is the result
>> relation and *foreign table*.
>
> I had been trying to work out an FDW-specific way to address this, but I
> think Dean's right that this should be addressed in
> expand_security_qual(), which means it'll apply to all cases and not
> just these FDW calls.  I don't think that's actually an issue though and
> it'll match up to how SELECT FOR UPDATE is handled today.

Sorry, my explanation was not accurate, but I also agree with Dean's 
idea.  In the above, I just wanted to make it clear that such a lock 
request done by expand_security_qual() should be limited to the case 
where the relation that is a former result relation is a foreign table.

If it's OK, I'll submit a patch for that, maybe early next week.

Thank you for working on this issue!

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Etsuro,

* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> On 2015/02/11 4:06, Stephen Frost wrote:
> >* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> >>On 2015/02/10 7:23, Dean Rasheed wrote:
> >>>Sorry, I didn't have time to look at this properly. My initial thought
> >>>is that expand_security_qual() needs to request a lock on rows coming
> >>>from the relation it pushes down into a subquery if that relation was
> >>>the result relation, because otherwise it won't have any locks, since
> >>>preprocess_rowmarks() only adds PlanRowMarks to non-target relations.
> >>
> >>That seems close to what I had in mind; expand_security_qual() needs
> >>to request a FOR UPDATE lock on rows coming from the relation it
> >>pushes down into a subquery only when that relation is the result
> >>relation and *foreign table*.
> >
> >I had been trying to work out an FDW-specific way to address this, but I
> >think Dean's right that this should be addressed in
> >expand_security_qual(), which means it'll apply to all cases and not
> >just these FDW calls.  I don't think that's actually an issue though and
> >it'll match up to how SELECT FOR UPDATE is handled today.
>
> Sorry, my explanation was not accurate, but I also agree with Dean's
> idea.  In the above, I just wanted to make it clear that such a lock
> request done by expand_security_qual() should be limited to the case
> where the relation that is a former result relation is a foreign
> table.

We aren't doing that for the other cases and so I don't think it makes
sense to do it here..  These should all be handled the same way.

> If it's OK, I'll submit a patch for that, maybe early next week.

Not really necessary, I have the code for it, just need to test, etc.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Etsuro,

* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> On 2015/02/11 4:06, Stephen Frost wrote:
> >I had been trying to work out an FDW-specific way to address this, but I
> >think Dean's right that this should be addressed in
> >expand_security_qual(), which means it'll apply to all cases and not
> >just these FDW calls.  I don't think that's actually an issue though and
> >it'll match up to how SELECT FOR UPDATE is handled today.
>
> Sorry, my explanation was not accurate, but I also agree with Dean's
> idea.  In the above, I just wanted to make it clear that such a lock
> request done by expand_security_qual() should be limited to the case
> where the relation that is a former result relation is a foreign
> table.

Attached is a patch which should address this.  Would love your (or
anyone else's) feedback on it.  It appears to address the issue which
you raised and the regression test changes are all in-line with
inserting a LockRows into the subquery, as anticipated.

    Thanks!

        Stephen

Вложения

Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/02/18 7:44, Stephen Frost wrote:
> * Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
>> On 2015/02/11 4:06, Stephen Frost wrote:
>>> I had been trying to work out an FDW-specific way to address this, but I
>>> think Dean's right that this should be addressed in
>>> expand_security_qual(), which means it'll apply to all cases and not
>>> just these FDW calls.  I don't think that's actually an issue though and
>>> it'll match up to how SELECT FOR UPDATE is handled today.
>>
>> Sorry, my explanation was not accurate, but I also agree with Dean's
>> idea.  In the above, I just wanted to make it clear that such a lock
>> request done by expand_security_qual() should be limited to the case
>> where the relation that is a former result relation is a foreign
>> table.
>
> Attached is a patch which should address this.  Would love your (or
> anyone else's) feedback on it.  It appears to address the issue which
> you raised and the regression test changes are all in-line with
> inserting a LockRows into the subquery, as anticipated.

I've looked into the patch.

* The patch applies to the latest head, 'make' passes successfully, but 
'make check' fails in the rowsecurity test.

* I found one place in expand_security_qual that I'm concerned about:

+            if (targetRelation)
+                applyLockingClause(subquery, 1, LCS_FORUPDATE,
+                                   false, false);

ISTM that it'd be better to use LockWaitBlock as the fourth argument of 
applyLockingClause.

Other than that, the patch looks good to me.

Thanks for the work!

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Etsuro,

* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> On 2015/02/18 7:44, Stephen Frost wrote:
> >Attached is a patch which should address this.  Would love your (or
> >anyone else's) feedback on it.  It appears to address the issue which
> >you raised and the regression test changes are all in-line with
> >inserting a LockRows into the subquery, as anticipated.
>
> I've looked into the patch.
>
> * The patch applies to the latest head, 'make' passes successfully,
> but 'make check' fails in the rowsecurity test.

Apologies for not being clear- the patch was against 9.4, where it
passes all the regression tests (at least for me- if you see
differently, please let me know!).

> * I found one place in expand_security_qual that I'm concerned about:
>
> +            if (targetRelation)
> +                applyLockingClause(subquery, 1, LCS_FORUPDATE,
> +                                   false, false);
>
> ISTM that it'd be better to use LockWaitBlock as the fourth argument
> of applyLockingClause.

LockWaitBlock isn't in 9.4. :)  Otherwise, I'd agree, and it's what I
plan to do for master.

> Other than that, the patch looks good to me.

Great, thanks!
Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Etsuro,

* Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
> On 2015/02/18 7:44, Stephen Frost wrote:
> * The patch applies to the latest head, 'make' passes successfully,
> but 'make check' fails in the rowsecurity test.

Here's the patch against master.  I'm still fiddling with the comment
wording and the commit message a bit, but barring objections these
patches are what I'm planning to move forward with.

    Thanks!

        Stephen

Вложения

Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/02/18 21:44, Stephen Frost wrote:
> * Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
>> On 2015/02/18 7:44, Stephen Frost wrote:
>>> Attached is a patch which should address this.  Would love your (or
>>> anyone else's) feedback on it.  It appears to address the issue which
>>> you raised and the regression test changes are all in-line with
>>> inserting a LockRows into the subquery, as anticipated.
>>
>> I've looked into the patch.
>>
>> * The patch applies to the latest head, 'make' passes successfully,
>> but 'make check' fails in the rowsecurity test.
>
> Apologies for not being clear- the patch was against 9.4, where it
> passes all the regression tests (at least for me- if you see
> differently, please let me know!).

Sorry, I assumed that the patch was against HEAD.  I comfermed that the 
back-patched 9.4 passes all the regression tests!

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Dean Rasheed
Дата:
On 18 February 2015 at 16:22, Stephen Frost <sfrost@snowman.net> wrote:
> Here's the patch against master.  I'm still fiddling with the comment
> wording and the commit message a bit, but barring objections these
> patches are what I'm planning to move forward with.
>

Yes, that matches what I had in mind.

While you're tweaking comments, you might want to look at the comment
in the block above which also relates to this new code, and says that
"we will end up locking all rows which pass the securityQuals". That's
not really accurate, I think it wants to say something like more like
"we won't necessarily be able to push user-defined quals down into the
subquery since they may include untrusted functions, and that means
that we may end up locking rows that don't pass the user-defined
quals.  In the worst case, we may end up locking all rows which pass
the securityQuals...".

Regards,
Dean



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Dean, Etsuro,

* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 18 February 2015 at 16:22, Stephen Frost <sfrost@snowman.net> wrote:
> > Here's the patch against master.  I'm still fiddling with the comment
> > wording and the commit message a bit, but barring objections these
> > patches are what I'm planning to move forward with.
> >
>
> Yes, that matches what I had in mind.
>
> While you're tweaking comments, you might want to look at the comment
> in the block above which also relates to this new code, and says that
> "we will end up locking all rows which pass the securityQuals". That's
> not really accurate, I think it wants to say something like more like
> "we won't necessarily be able to push user-defined quals down into the
> subquery since they may include untrusted functions, and that means
> that we may end up locking rows that don't pass the user-defined
> quals.  In the worst case, we may end up locking all rows which pass
> the securityQuals...".

I've pushed an update for this to master and 9.4 and improved the
comments and the commit message as discussed.

Would be great if you could test and let me know if you run into any
issues!
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/02/26 11:38, Stephen Frost wrote:
> I've pushed an update for this to master and 9.4 and improved the
> comments and the commit message as discussed.
>
> Would be great if you could test and let me know if you run into any
> issues!

OK, thanks!

Best regards,
Etsuro Fujita



Re: Odd behavior of updatable security barrier views on foreign tables

От
Dean Rasheed
Дата:
On 27 February 2015 at 03:10, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
> On 2015/02/26 11:38, Stephen Frost wrote:
>>
>> I've pushed an update for this to master and 9.4 and improved the
>> comments and the commit message as discussed.
>>
>> Would be great if you could test and let me know if you run into any
>> issues!
>

I just spotted a trivial bug in this patch -- in
expand_security_quals() you need to set targetRelation = false inside
the loop, otherwise it will be true for the target relation and all
that follow it. That was why the regression test output from
rls.v4.patch on the other thread wasn't what I expected.

Regards,
Dean



Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 27 February 2015 at 03:10, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
> > On 2015/02/26 11:38, Stephen Frost wrote:
> >>
> >> I've pushed an update for this to master and 9.4 and improved the
> >> comments and the commit message as discussed.
> >>
> >> Would be great if you could test and let me know if you run into any
> >> issues!
>
> I just spotted a trivial bug in this patch -- in
> expand_security_quals() you need to set targetRelation = false inside
> the loop, otherwise it will be true for the target relation and all
> that follow it. That was why the regression test output from
> rls.v4.patch on the other thread wasn't what I expected.

Err, I thought it was initialized at the top of that loop back to
false..  Will take a look shortly.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> On 27 February 2015 at 03:10, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
> > On 2015/02/26 11:38, Stephen Frost wrote:
> >>
> >> I've pushed an update for this to master and 9.4 and improved the
> >> comments and the commit message as discussed.
> >>
> >> Would be great if you could test and let me know if you run into any
> >> issues!
> >
>
> I just spotted a trivial bug in this patch -- in
> expand_security_quals() you need to set targetRelation = false inside
> the loop, otherwise it will be true for the target relation and all
> that follow it. That was why the regression test output from
> rls.v4.patch on the other thread wasn't what I expected.

Wow, no, it's done at the entry to the function.  I really thought that
was defined and initialized inside the foreach()..  That was certainly
my intent.

Will fix, many thanks!
Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Stephen Frost
Дата:
Dean,

* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
> I just spotted a trivial bug in this patch -- in
> expand_security_quals() you need to set targetRelation = false inside
> the loop, otherwise it will be true for the target relation and all
> that follow it.

I've pushed a fix for this.
Thanks!
    Stephen

Re: Odd behavior of updatable security barrier views on foreign tables

От
Etsuro Fujita
Дата:
On 2015/03/02 5:28, Stephen Frost wrote:
> * Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
>> I just spotted a trivial bug in this patch -- in
>> expand_security_quals() you need to set targetRelation = false inside
>> the loop, otherwise it will be true for the target relation and all
>> that follow it.

> I've pushed a fix for this.

Thanks!

Best regards,
Etsuro Fujita