Odd behavior of updatable security barrier views on foreign tables
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
-> 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.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* 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
* 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
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
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
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
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
Attachments:
fix-sbv-locking-9.4.patchtext/x-diff; charset=us-asciiDownload
From 0719cbb3b2b4c6bc1c7f52f825f1e14ec27c4b7b Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Tue, 17 Feb 2015 15:43:33 -0500
Subject: [PATCH] Add locking clause for SB views for update/delete
In expand_security_qual(), we were handling locking correctly when a
PlanRowMark existed, but not when we were working with the target
relation (which doesn't have any PlanRowMarks, but the subquery created
for the security barrier quals still needs to lock the rows under it).
Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't
properly issuing a SELECT ... FOR UPDATE to the remote side under a
DELETE.
Back-patch to 9.4 where updatable security barrier views were
introduced.
---
src/backend/optimizer/prep/prepsecurity.c | 24 ++-
src/test/regress/expected/updatable_views.out | 264 ++++++++++++++------------
2 files changed, 163 insertions(+), 125 deletions(-)
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
index 51f10a4..bb5c397 100644
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -37,7 +37,7 @@ typedef struct
} security_barrier_replace_vars_context;
static void expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
- RangeTblEntry *rte, Node *qual);
+ RangeTblEntry *rte, Node *qual, bool targetRelation);
static void security_barrier_replace_vars(Node *node,
security_barrier_replace_vars_context *context);
@@ -63,6 +63,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
Query *parse = root->parse;
int rt_index;
ListCell *cell;
+ bool targetRelation = false;
/*
* Process each RTE in the rtable list.
@@ -98,6 +99,12 @@ expand_security_quals(PlannerInfo *root, List *tlist)
{
RangeTblEntry *newrte = copyObject(rte);
+ /*
+ * We need to let expand_security_qual know if this is the target
+ * relation, as it has additional work to do in that case.
+ */
+ targetRelation = true;
+
parse->rtable = lappend(parse->rtable, newrte);
parse->resultRelation = list_length(parse->rtable);
@@ -147,7 +154,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
rte->securityQuals = list_delete_first(rte->securityQuals);
ChangeVarNodes(qual, rt_index, 1, 0);
- expand_security_qual(root, tlist, rt_index, rte, qual);
+ expand_security_qual(root, tlist, rt_index, rte, qual,
+ targetRelation);
}
}
}
@@ -160,7 +168,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
*/
static void
expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
- RangeTblEntry *rte, Node *qual)
+ RangeTblEntry *rte, Node *qual, bool targetRelation)
{
Query *parse = root->parse;
Oid relid = rte->relid;
@@ -256,6 +264,16 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
}
/*
+ * We need to handle the case where this is the target relation
+ * explicitly since it won't have any row marks, because we still
+ * need to lock the records coming back from the with-security-quals
+ * subquery. This might not appear obivous, but it matches what
+ * we're doing above and keeps FDWs happy too.
+ */
+ if (targetRelation)
+ applyLockingClause(subquery, 1, LCS_FORUPDATE,
+ false, false);
+ /*
* Replace any variables in the outer query that refer to the
* original relation RTE with references to columns that we will
* expose in the new subquery, building the subquery's targetlist
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 507b6a2..a1d03f3 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -1842,24 +1842,26 @@ EXPLAIN (costs off) SELECT * FROM rw_view1 WHERE snoop(person);
(4 rows)
EXPLAIN (costs off) UPDATE rw_view1 SET person=person WHERE snoop(person);
- QUERY PLAN
------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------
Update on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: snoop(base_tbl.person)
- -> Seq Scan on base_tbl base_tbl_2
- Filter: (visibility = 'public'::text)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_2
+ Filter: (visibility = 'public'::text)
+(6 rows)
EXPLAIN (costs off) DELETE FROM rw_view1 WHERE NOT snoop(person);
- QUERY PLAN
------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------
Delete on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: (NOT snoop(base_tbl.person))
- -> Seq Scan on base_tbl base_tbl_2
- Filter: (visibility = 'public'::text)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_2
+ Filter: (visibility = 'public'::text)
+(6 rows)
-- security barrier view on top of security barrier view
CREATE VIEW rw_view2 WITH (security_barrier = true) AS
@@ -1922,28 +1924,30 @@ EXPLAIN (costs off) SELECT * FROM rw_view2 WHERE snoop(person);
(6 rows)
EXPLAIN (costs off) UPDATE rw_view2 SET person=person WHERE snoop(person);
- QUERY PLAN
------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------
Update on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: snoop(base_tbl.person)
-> Subquery Scan on base_tbl_2
Filter: snoop(base_tbl_2.person)
- -> Seq Scan on base_tbl base_tbl_3
- Filter: (visibility = 'public'::text)
-(7 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_3
+ Filter: (visibility = 'public'::text)
+(8 rows)
EXPLAIN (costs off) DELETE FROM rw_view2 WHERE NOT snoop(person);
- QUERY PLAN
------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------
Delete on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: (NOT snoop(base_tbl.person))
-> Subquery Scan on base_tbl_2
Filter: snoop(base_tbl_2.person)
- -> Seq Scan on base_tbl base_tbl_3
- Filter: (visibility = 'public'::text)
-(7 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_3
+ Filter: (visibility = 'public'::text)
+(8 rows)
DROP TABLE base_tbl CASCADE;
NOTICE: drop cascades to 2 other objects
@@ -2057,70 +2061,78 @@ SELECT * FROM v1 WHERE a=8;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a = 3;
- QUERY PLAN
--------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------
Update on public.t1 t1_4
-> Subquery Scan on t1
Output: 100, t1.b, t1.c, t1.ctid
Filter: snoop(t1.a)
- -> Nested Loop Semi Join
- Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
- -> Seq Scan on public.t1 t1_5
- Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
- Filter: ((t1_5.a > 5) AND (t1_5.a = 3) AND leakproof(t1_5.a))
- -> Append
- -> Seq Scan on public.t12
- Output: t12.a
- Filter: (t12.a = 3)
- -> Seq Scan on public.t111
- Output: t111.a
- Filter: (t111.a = 3)
+ -> LockRows
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Nested Loop Semi Join
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Seq Scan on public.t1 t1_5
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
+ Filter: ((t1_5.a > 5) AND (t1_5.a = 3) AND leakproof(t1_5.a))
+ -> Append
+ -> Seq Scan on public.t12
+ Output: t12.ctid, t12.tableoid, t12.a
+ Filter: (t12.a = 3)
+ -> Seq Scan on public.t111
+ Output: t111.ctid, t111.tableoid, t111.a
+ Filter: (t111.a = 3)
-> Subquery Scan on t1_1
Output: 100, t1_1.b, t1_1.c, t1_1.d, t1_1.ctid
Filter: snoop(t1_1.a)
- -> Nested Loop Semi Join
- Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
- -> Seq Scan on public.t11
- Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
- Filter: ((t11.a > 5) AND (t11.a = 3) AND leakproof(t11.a))
- -> Append
- -> Seq Scan on public.t12 t12_1
- Output: t12_1.a
- Filter: (t12_1.a = 3)
- -> Seq Scan on public.t111 t111_1
- Output: t111_1.a
- Filter: (t111_1.a = 3)
+ -> LockRows
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Nested Loop Semi Join
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Seq Scan on public.t11
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
+ Filter: ((t11.a > 5) AND (t11.a = 3) AND leakproof(t11.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_1
+ Output: t12_1.ctid, t12_1.tableoid, t12_1.a
+ Filter: (t12_1.a = 3)
+ -> Seq Scan on public.t111 t111_1
+ Output: t111_1.ctid, t111_1.tableoid, t111_1.a
+ Filter: (t111_1.a = 3)
-> Subquery Scan on t1_2
Output: 100, t1_2.b, t1_2.c, t1_2.e, t1_2.ctid
Filter: snoop(t1_2.a)
- -> Nested Loop Semi Join
- Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
- -> Seq Scan on public.t12 t12_2
- Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
- Filter: ((t12_2.a > 5) AND (t12_2.a = 3) AND leakproof(t12_2.a))
- -> Append
- -> Seq Scan on public.t12 t12_3
- Output: t12_3.a
- Filter: (t12_3.a = 3)
- -> Seq Scan on public.t111 t111_2
- Output: t111_2.a
- Filter: (t111_2.a = 3)
+ -> LockRows
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Nested Loop Semi Join
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Seq Scan on public.t12 t12_2
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
+ Filter: ((t12_2.a > 5) AND (t12_2.a = 3) AND leakproof(t12_2.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_3
+ Output: t12_3.ctid, t12_3.tableoid, t12_3.a
+ Filter: (t12_3.a = 3)
+ -> Seq Scan on public.t111 t111_2
+ Output: t111_2.ctid, t111_2.tableoid, t111_2.a
+ Filter: (t111_2.a = 3)
-> Subquery Scan on t1_3
Output: 100, t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid
Filter: snoop(t1_3.a)
- -> Nested Loop Semi Join
- Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- -> Seq Scan on public.t111 t111_3
- Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- Filter: ((t111_3.a > 5) AND (t111_3.a = 3) AND leakproof(t111_3.a))
- -> Append
- -> Seq Scan on public.t12 t12_4
- Output: t12_4.a
- Filter: (t12_4.a = 3)
- -> Seq Scan on public.t111 t111_4
- Output: t111_4.a
- Filter: (t111_4.a = 3)
-(61 rows)
+ -> LockRows
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Nested Loop Semi Join
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Seq Scan on public.t111 t111_3
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
+ Filter: ((t111_3.a > 5) AND (t111_3.a = 3) AND leakproof(t111_3.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_4
+ Output: t12_4.ctid, t12_4.tableoid, t12_4.a
+ Filter: (t12_4.a = 3)
+ -> Seq Scan on public.t111 t111_4
+ Output: t111_4.ctid, t111_4.tableoid, t111_4.a
+ Filter: (t111_4.a = 3)
+(69 rows)
UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a = 3;
SELECT * FROM v1 WHERE a=100; -- Nothing should have been changed to 100
@@ -2135,70 +2147,78 @@ SELECT * FROM t1 WHERE a=100; -- Nothing should have been changed to 100
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
- QUERY PLAN
--------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------
Update on public.t1 t1_4
-> Subquery Scan on t1
Output: (t1.a + 1), t1.b, t1.c, t1.ctid
Filter: snoop(t1.a)
- -> Nested Loop Semi Join
- Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
- -> Seq Scan on public.t1 t1_5
- Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
- Filter: ((t1_5.a > 5) AND (t1_5.a = 8) AND leakproof(t1_5.a))
- -> Append
- -> Seq Scan on public.t12
- Output: t12.a
- Filter: (t12.a = 8)
- -> Seq Scan on public.t111
- Output: t111.a
- Filter: (t111.a = 8)
+ -> LockRows
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Nested Loop Semi Join
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Seq Scan on public.t1 t1_5
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
+ Filter: ((t1_5.a > 5) AND (t1_5.a = 8) AND leakproof(t1_5.a))
+ -> Append
+ -> Seq Scan on public.t12
+ Output: t12.ctid, t12.tableoid, t12.a
+ Filter: (t12.a = 8)
+ -> Seq Scan on public.t111
+ Output: t111.ctid, t111.tableoid, t111.a
+ Filter: (t111.a = 8)
-> Subquery Scan on t1_1
Output: (t1_1.a + 1), t1_1.b, t1_1.c, t1_1.d, t1_1.ctid
Filter: snoop(t1_1.a)
- -> Nested Loop Semi Join
- Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
- -> Seq Scan on public.t11
- Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
- Filter: ((t11.a > 5) AND (t11.a = 8) AND leakproof(t11.a))
- -> Append
- -> Seq Scan on public.t12 t12_1
- Output: t12_1.a
- Filter: (t12_1.a = 8)
- -> Seq Scan on public.t111 t111_1
- Output: t111_1.a
- Filter: (t111_1.a = 8)
+ -> LockRows
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Nested Loop Semi Join
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Seq Scan on public.t11
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
+ Filter: ((t11.a > 5) AND (t11.a = 8) AND leakproof(t11.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_1
+ Output: t12_1.ctid, t12_1.tableoid, t12_1.a
+ Filter: (t12_1.a = 8)
+ -> Seq Scan on public.t111 t111_1
+ Output: t111_1.ctid, t111_1.tableoid, t111_1.a
+ Filter: (t111_1.a = 8)
-> Subquery Scan on t1_2
Output: (t1_2.a + 1), t1_2.b, t1_2.c, t1_2.e, t1_2.ctid
Filter: snoop(t1_2.a)
- -> Nested Loop Semi Join
- Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
- -> Seq Scan on public.t12 t12_2
- Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
- Filter: ((t12_2.a > 5) AND (t12_2.a = 8) AND leakproof(t12_2.a))
- -> Append
- -> Seq Scan on public.t12 t12_3
- Output: t12_3.a
- Filter: (t12_3.a = 8)
- -> Seq Scan on public.t111 t111_2
- Output: t111_2.a
- Filter: (t111_2.a = 8)
+ -> LockRows
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Nested Loop Semi Join
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Seq Scan on public.t12 t12_2
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
+ Filter: ((t12_2.a > 5) AND (t12_2.a = 8) AND leakproof(t12_2.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_3
+ Output: t12_3.ctid, t12_3.tableoid, t12_3.a
+ Filter: (t12_3.a = 8)
+ -> Seq Scan on public.t111 t111_2
+ Output: t111_2.ctid, t111_2.tableoid, t111_2.a
+ Filter: (t111_2.a = 8)
-> Subquery Scan on t1_3
Output: (t1_3.a + 1), t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid
Filter: snoop(t1_3.a)
- -> Nested Loop Semi Join
- Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- -> Seq Scan on public.t111 t111_3
- Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- Filter: ((t111_3.a > 5) AND (t111_3.a = 8) AND leakproof(t111_3.a))
- -> Append
- -> Seq Scan on public.t12 t12_4
- Output: t12_4.a
- Filter: (t12_4.a = 8)
- -> Seq Scan on public.t111 t111_4
- Output: t111_4.a
- Filter: (t111_4.a = 8)
-(61 rows)
+ -> LockRows
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Nested Loop Semi Join
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Seq Scan on public.t111 t111_3
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
+ Filter: ((t111_3.a > 5) AND (t111_3.a = 8) AND leakproof(t111_3.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_4
+ Output: t12_4.ctid, t12_4.tableoid, t12_4.a
+ Filter: (t12_4.a = 8)
+ -> Seq Scan on public.t111 t111_4
+ Output: t111_4.ctid, t111_4.tableoid, t111_4.a
+ Filter: (t111_4.a = 8)
+(69 rows)
UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
NOTICE: snooped value: 8
--
1.9.1
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
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
Attachments:
fix-sbv-locking-master.patchtext/x-diff; charset=us-asciiDownload
From ea3713a8b648459d3024d331ef0374f6c9622247 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Tue, 17 Feb 2015 15:43:33 -0500
Subject: [PATCH] Add locking clause for SB views for update/delete
In expand_security_qual(), we were handling locking correctly when a
PlanRowMark existed, but not when we were working with the target
relation (which doesn't have any PlanRowMarks, but the subquery created
for the security barrier quals still needs to lock the rows under it).
Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't
properly issuing a SELECT ... FOR UPDATE to the remote side under a
DELETE.
Back-patch to 9.4 where updatable security barrier views were
introduced.
---
src/backend/optimizer/prep/prepsecurity.c | 24 ++-
src/test/regress/expected/rowsecurity.out | 64 ++++---
src/test/regress/expected/updatable_views.out | 264 ++++++++++++++------------
3 files changed, 199 insertions(+), 153 deletions(-)
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
index af3ee61..ce7b203 100644
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -37,7 +37,7 @@ typedef struct
} security_barrier_replace_vars_context;
static void expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
- RangeTblEntry *rte, Node *qual);
+ RangeTblEntry *rte, Node *qual, bool targetRelation);
static void security_barrier_replace_vars(Node *node,
security_barrier_replace_vars_context *context);
@@ -63,6 +63,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
Query *parse = root->parse;
int rt_index;
ListCell *cell;
+ bool targetRelation = false;
/*
* Process each RTE in the rtable list.
@@ -98,6 +99,12 @@ expand_security_quals(PlannerInfo *root, List *tlist)
{
RangeTblEntry *newrte = copyObject(rte);
+ /*
+ * We need to let expand_security_qual know if this is the target
+ * relation, as it has additional work to do in that case.
+ */
+ targetRelation = true;
+
parse->rtable = lappend(parse->rtable, newrte);
parse->resultRelation = list_length(parse->rtable);
@@ -147,7 +154,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
rte->securityQuals = list_delete_first(rte->securityQuals);
ChangeVarNodes(qual, rt_index, 1, 0);
- expand_security_qual(root, tlist, rt_index, rte, qual);
+ expand_security_qual(root, tlist, rt_index, rte, qual,
+ targetRelation);
}
}
}
@@ -160,7 +168,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
*/
static void
expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
- RangeTblEntry *rte, Node *qual)
+ RangeTblEntry *rte, Node *qual, bool targetRelation)
{
Query *parse = root->parse;
Oid relid = rte->relid;
@@ -256,6 +264,16 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
}
/*
+ * We need to handle the case where this is the target relation
+ * explicitly since it won't have any row marks, because we still
+ * need to lock the records coming back from the with-security-quals
+ * subquery. This might not appear obivous, but it matches what
+ * we're doing above and keeps FDWs happy too.
+ */
+ if (targetRelation)
+ applyLockingClause(subquery, 1, LCS_FORUPDATE,
+ LockWaitBlock, false);
+ /*
* Replace any variables in the outer query that refer to the
* original relation RTE with references to columns that we will
* expose in the new subquery, building the subquery's targetlist
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 21817d8..f41bef1 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1034,22 +1034,25 @@ EXPLAIN (COSTS OFF) EXECUTE p2(2);
--
SET SESSION AUTHORIZATION rls_regress_user1;
EXPLAIN (COSTS OFF) UPDATE t1 SET b = b || b WHERE f_leak(b);
- QUERY PLAN
--------------------------------------
+ QUERY PLAN
+-------------------------------------------
Update on t1 t1_3
-> Subquery Scan on t1
Filter: f_leak(t1.b)
- -> Seq Scan on t1 t1_4
- Filter: ((a % 2) = 0)
+ -> LockRows
+ -> Seq Scan on t1 t1_4
+ Filter: ((a % 2) = 0)
-> Subquery Scan on t1_1
Filter: f_leak(t1_1.b)
- -> Seq Scan on t2
- Filter: ((a % 2) = 0)
+ -> LockRows
+ -> Seq Scan on t2
+ Filter: ((a % 2) = 0)
-> Subquery Scan on t1_2
Filter: f_leak(t1_2.b)
- -> Seq Scan on t3
- Filter: ((a % 2) = 0)
-(13 rows)
+ -> LockRows
+ -> Seq Scan on t3
+ Filter: ((a % 2) = 0)
+(16 rows)
UPDATE t1 SET b = b || b WHERE f_leak(b);
NOTICE: f_leak => bbb
@@ -1058,14 +1061,15 @@ NOTICE: f_leak => bcd
NOTICE: f_leak => def
NOTICE: f_leak => yyy
EXPLAIN (COSTS OFF) UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b);
- QUERY PLAN
--------------------------------------
+ QUERY PLAN
+-------------------------------------------
Update on t1 t1_1
-> Subquery Scan on t1
Filter: f_leak(t1.b)
- -> Seq Scan on t1 t1_2
- Filter: ((a % 2) = 0)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on t1 t1_2
+ Filter: ((a % 2) = 0)
+(6 rows)
UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b);
NOTICE: f_leak => bbbbbb
@@ -1131,32 +1135,36 @@ SELECT * FROM t1;
SET SESSION AUTHORIZATION rls_regress_user1;
SET row_security TO ON;
EXPLAIN (COSTS OFF) DELETE FROM only t1 WHERE f_leak(b);
- QUERY PLAN
--------------------------------------
+ QUERY PLAN
+-------------------------------------------
Delete on t1 t1_1
-> Subquery Scan on t1
Filter: f_leak(t1.b)
- -> Seq Scan on t1 t1_2
- Filter: ((a % 2) = 0)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on t1 t1_2
+ Filter: ((a % 2) = 0)
+(6 rows)
EXPLAIN (COSTS OFF) DELETE FROM t1 WHERE f_leak(b);
- QUERY PLAN
--------------------------------------
+ QUERY PLAN
+-------------------------------------------
Delete on t1 t1_3
-> Subquery Scan on t1
Filter: f_leak(t1.b)
- -> Seq Scan on t1 t1_4
- Filter: ((a % 2) = 0)
+ -> LockRows
+ -> Seq Scan on t1 t1_4
+ Filter: ((a % 2) = 0)
-> Subquery Scan on t1_1
Filter: f_leak(t1_1.b)
- -> Seq Scan on t2
- Filter: ((a % 2) = 0)
+ -> LockRows
+ -> Seq Scan on t2
+ Filter: ((a % 2) = 0)
-> Subquery Scan on t1_2
Filter: f_leak(t1_2.b)
- -> Seq Scan on t3
- Filter: ((a % 2) = 0)
-(13 rows)
+ -> LockRows
+ -> Seq Scan on t3
+ Filter: ((a % 2) = 0)
+(16 rows)
DELETE FROM only t1 WHERE f_leak(b) RETURNING oid, *, t1;
NOTICE: f_leak => bbbbbb_updt
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 80c5706..c49e769 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -1842,24 +1842,26 @@ EXPLAIN (costs off) SELECT * FROM rw_view1 WHERE snoop(person);
(4 rows)
EXPLAIN (costs off) UPDATE rw_view1 SET person=person WHERE snoop(person);
- QUERY PLAN
------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------
Update on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: snoop(base_tbl.person)
- -> Seq Scan on base_tbl base_tbl_2
- Filter: (visibility = 'public'::text)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_2
+ Filter: (visibility = 'public'::text)
+(6 rows)
EXPLAIN (costs off) DELETE FROM rw_view1 WHERE NOT snoop(person);
- QUERY PLAN
------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------
Delete on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: (NOT snoop(base_tbl.person))
- -> Seq Scan on base_tbl base_tbl_2
- Filter: (visibility = 'public'::text)
-(5 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_2
+ Filter: (visibility = 'public'::text)
+(6 rows)
-- security barrier view on top of security barrier view
CREATE VIEW rw_view2 WITH (security_barrier = true) AS
@@ -1922,28 +1924,30 @@ EXPLAIN (costs off) SELECT * FROM rw_view2 WHERE snoop(person);
(6 rows)
EXPLAIN (costs off) UPDATE rw_view2 SET person=person WHERE snoop(person);
- QUERY PLAN
------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------
Update on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: snoop(base_tbl.person)
-> Subquery Scan on base_tbl_2
Filter: snoop(base_tbl_2.person)
- -> Seq Scan on base_tbl base_tbl_3
- Filter: (visibility = 'public'::text)
-(7 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_3
+ Filter: (visibility = 'public'::text)
+(8 rows)
EXPLAIN (costs off) DELETE FROM rw_view2 WHERE NOT snoop(person);
- QUERY PLAN
------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------
Delete on base_tbl base_tbl_1
-> Subquery Scan on base_tbl
Filter: (NOT snoop(base_tbl.person))
-> Subquery Scan on base_tbl_2
Filter: snoop(base_tbl_2.person)
- -> Seq Scan on base_tbl base_tbl_3
- Filter: (visibility = 'public'::text)
-(7 rows)
+ -> LockRows
+ -> Seq Scan on base_tbl base_tbl_3
+ Filter: (visibility = 'public'::text)
+(8 rows)
DROP TABLE base_tbl CASCADE;
NOTICE: drop cascades to 2 other objects
@@ -2057,70 +2061,78 @@ SELECT * FROM v1 WHERE a=8;
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a = 3;
- QUERY PLAN
--------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------
Update on public.t1 t1_4
-> Subquery Scan on t1
Output: 100, t1.b, t1.c, t1.ctid
Filter: snoop(t1.a)
- -> Nested Loop Semi Join
- Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
- -> Seq Scan on public.t1 t1_5
- Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
- Filter: ((t1_5.a > 5) AND (t1_5.a = 3) AND leakproof(t1_5.a))
- -> Append
- -> Seq Scan on public.t12
- Output: t12.a
- Filter: (t12.a = 3)
- -> Seq Scan on public.t111
- Output: t111.a
- Filter: (t111.a = 3)
+ -> LockRows
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Nested Loop Semi Join
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Seq Scan on public.t1 t1_5
+ Output: t1_5.ctid, t1_5.a, t1_5.b, t1_5.c
+ Filter: ((t1_5.a > 5) AND (t1_5.a = 3) AND leakproof(t1_5.a))
+ -> Append
+ -> Seq Scan on public.t12
+ Output: t12.ctid, t12.tableoid, t12.a
+ Filter: (t12.a = 3)
+ -> Seq Scan on public.t111
+ Output: t111.ctid, t111.tableoid, t111.a
+ Filter: (t111.a = 3)
-> Subquery Scan on t1_1
Output: 100, t1_1.b, t1_1.c, t1_1.d, t1_1.ctid
Filter: snoop(t1_1.a)
- -> Nested Loop Semi Join
- Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
- -> Seq Scan on public.t11
- Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
- Filter: ((t11.a > 5) AND (t11.a = 3) AND leakproof(t11.a))
- -> Append
- -> Seq Scan on public.t12 t12_1
- Output: t12_1.a
- Filter: (t12_1.a = 3)
- -> Seq Scan on public.t111 t111_1
- Output: t111_1.a
- Filter: (t111_1.a = 3)
+ -> LockRows
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Nested Loop Semi Join
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Seq Scan on public.t11
+ Output: t11.ctid, t11.a, t11.b, t11.c, t11.d
+ Filter: ((t11.a > 5) AND (t11.a = 3) AND leakproof(t11.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_1
+ Output: t12_1.ctid, t12_1.tableoid, t12_1.a
+ Filter: (t12_1.a = 3)
+ -> Seq Scan on public.t111 t111_1
+ Output: t111_1.ctid, t111_1.tableoid, t111_1.a
+ Filter: (t111_1.a = 3)
-> Subquery Scan on t1_2
Output: 100, t1_2.b, t1_2.c, t1_2.e, t1_2.ctid
Filter: snoop(t1_2.a)
- -> Nested Loop Semi Join
- Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
- -> Seq Scan on public.t12 t12_2
- Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
- Filter: ((t12_2.a > 5) AND (t12_2.a = 3) AND leakproof(t12_2.a))
- -> Append
- -> Seq Scan on public.t12 t12_3
- Output: t12_3.a
- Filter: (t12_3.a = 3)
- -> Seq Scan on public.t111 t111_2
- Output: t111_2.a
- Filter: (t111_2.a = 3)
+ -> LockRows
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Nested Loop Semi Join
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Seq Scan on public.t12 t12_2
+ Output: t12_2.ctid, t12_2.a, t12_2.b, t12_2.c, t12_2.e
+ Filter: ((t12_2.a > 5) AND (t12_2.a = 3) AND leakproof(t12_2.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_3
+ Output: t12_3.ctid, t12_3.tableoid, t12_3.a
+ Filter: (t12_3.a = 3)
+ -> Seq Scan on public.t111 t111_2
+ Output: t111_2.ctid, t111_2.tableoid, t111_2.a
+ Filter: (t111_2.a = 3)
-> Subquery Scan on t1_3
Output: 100, t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid
Filter: snoop(t1_3.a)
- -> Nested Loop Semi Join
- Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- -> Seq Scan on public.t111 t111_3
- Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- Filter: ((t111_3.a > 5) AND (t111_3.a = 3) AND leakproof(t111_3.a))
- -> Append
- -> Seq Scan on public.t12 t12_4
- Output: t12_4.a
- Filter: (t12_4.a = 3)
- -> Seq Scan on public.t111 t111_4
- Output: t111_4.a
- Filter: (t111_4.a = 3)
-(61 rows)
+ -> LockRows
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Nested Loop Semi Join
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Seq Scan on public.t111 t111_3
+ Output: t111_3.ctid, t111_3.a, t111_3.b, t111_3.c, t111_3.d, t111_3.e
+ Filter: ((t111_3.a > 5) AND (t111_3.a = 3) AND leakproof(t111_3.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_4
+ Output: t12_4.ctid, t12_4.tableoid, t12_4.a
+ Filter: (t12_4.a = 3)
+ -> Seq Scan on public.t111 t111_4
+ Output: t111_4.ctid, t111_4.tableoid, t111_4.a
+ Filter: (t111_4.a = 3)
+(69 rows)
UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a = 3;
SELECT * FROM v1 WHERE a=100; -- Nothing should have been changed to 100
@@ -2135,70 +2147,78 @@ SELECT * FROM t1 WHERE a=100; -- Nothing should have been changed to 100
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
- QUERY PLAN
--------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------
Update on public.t1 t1_4
-> Subquery Scan on t1
Output: (t1.a + 1), t1.b, t1.c, t1.ctid
Filter: snoop(t1.a)
- -> Nested Loop Semi Join
- Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
- -> Seq Scan on public.t1 t1_5
- Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
- Filter: ((t1_5.a > 5) AND (t1_5.a = 8) AND leakproof(t1_5.a))
- -> Append
- -> Seq Scan on public.t12
- Output: t12.a
- Filter: (t12.a = 8)
- -> Seq Scan on public.t111
- Output: t111.a
- Filter: (t111.a = 8)
+ -> LockRows
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Nested Loop Semi Join
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c, t1_5.ctid, t12.ctid, t12.tableoid
+ -> Seq Scan on public.t1 t1_5
+ Output: t1_5.a, t1_5.ctid, t1_5.b, t1_5.c
+ Filter: ((t1_5.a > 5) AND (t1_5.a = 8) AND leakproof(t1_5.a))
+ -> Append
+ -> Seq Scan on public.t12
+ Output: t12.ctid, t12.tableoid, t12.a
+ Filter: (t12.a = 8)
+ -> Seq Scan on public.t111
+ Output: t111.ctid, t111.tableoid, t111.a
+ Filter: (t111.a = 8)
-> Subquery Scan on t1_1
Output: (t1_1.a + 1), t1_1.b, t1_1.c, t1_1.d, t1_1.ctid
Filter: snoop(t1_1.a)
- -> Nested Loop Semi Join
- Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
- -> Seq Scan on public.t11
- Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
- Filter: ((t11.a > 5) AND (t11.a = 8) AND leakproof(t11.a))
- -> Append
- -> Seq Scan on public.t12 t12_1
- Output: t12_1.a
- Filter: (t12_1.a = 8)
- -> Seq Scan on public.t111 t111_1
- Output: t111_1.a
- Filter: (t111_1.a = 8)
+ -> LockRows
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Nested Loop Semi Join
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d, t11.ctid, t12_1.ctid, t12_1.tableoid
+ -> Seq Scan on public.t11
+ Output: t11.a, t11.ctid, t11.b, t11.c, t11.d
+ Filter: ((t11.a > 5) AND (t11.a = 8) AND leakproof(t11.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_1
+ Output: t12_1.ctid, t12_1.tableoid, t12_1.a
+ Filter: (t12_1.a = 8)
+ -> Seq Scan on public.t111 t111_1
+ Output: t111_1.ctid, t111_1.tableoid, t111_1.a
+ Filter: (t111_1.a = 8)
-> Subquery Scan on t1_2
Output: (t1_2.a + 1), t1_2.b, t1_2.c, t1_2.e, t1_2.ctid
Filter: snoop(t1_2.a)
- -> Nested Loop Semi Join
- Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
- -> Seq Scan on public.t12 t12_2
- Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
- Filter: ((t12_2.a > 5) AND (t12_2.a = 8) AND leakproof(t12_2.a))
- -> Append
- -> Seq Scan on public.t12 t12_3
- Output: t12_3.a
- Filter: (t12_3.a = 8)
- -> Seq Scan on public.t111 t111_2
- Output: t111_2.a
- Filter: (t111_2.a = 8)
+ -> LockRows
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Nested Loop Semi Join
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e, t12_2.ctid, t12_3.ctid, t12_3.tableoid
+ -> Seq Scan on public.t12 t12_2
+ Output: t12_2.a, t12_2.ctid, t12_2.b, t12_2.c, t12_2.e
+ Filter: ((t12_2.a > 5) AND (t12_2.a = 8) AND leakproof(t12_2.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_3
+ Output: t12_3.ctid, t12_3.tableoid, t12_3.a
+ Filter: (t12_3.a = 8)
+ -> Seq Scan on public.t111 t111_2
+ Output: t111_2.ctid, t111_2.tableoid, t111_2.a
+ Filter: (t111_2.a = 8)
-> Subquery Scan on t1_3
Output: (t1_3.a + 1), t1_3.b, t1_3.c, t1_3.d, t1_3.e, t1_3.ctid
Filter: snoop(t1_3.a)
- -> Nested Loop Semi Join
- Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- -> Seq Scan on public.t111 t111_3
- Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
- Filter: ((t111_3.a > 5) AND (t111_3.a = 8) AND leakproof(t111_3.a))
- -> Append
- -> Seq Scan on public.t12 t12_4
- Output: t12_4.a
- Filter: (t12_4.a = 8)
- -> Seq Scan on public.t111 t111_4
- Output: t111_4.a
- Filter: (t111_4.a = 8)
-(61 rows)
+ -> LockRows
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Nested Loop Semi Join
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e, t111_3.ctid, t12_4.ctid, t12_4.tableoid
+ -> Seq Scan on public.t111 t111_3
+ Output: t111_3.a, t111_3.ctid, t111_3.b, t111_3.c, t111_3.d, t111_3.e
+ Filter: ((t111_3.a > 5) AND (t111_3.a = 8) AND leakproof(t111_3.a))
+ -> Append
+ -> Seq Scan on public.t12 t12_4
+ Output: t12_4.ctid, t12_4.tableoid, t12_4.a
+ Filter: (t12_4.a = 8)
+ -> Seq Scan on public.t111 t111_4
+ Output: t111_4.ctid, t111_4.tableoid, t111_4.a
+ Filter: (t111_4.a = 8)
+(69 rows)
UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
NOTICE: snooped value: 8
--
1.9.1
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* 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
* 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
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
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers