On 8 October 2015 at 05:45, Haribabu Kommi <kommi.harib...@gmail.com> wrote:
> On Thu, Oct 8, 2015 at 2:54 PM, Stephen Frost <sfr...@snowman.net> wrote:
>> It's quite late here, but I'll take a look at this in more depth
>> tomorrow.
>> Based on what the Assert's testing, I took an educated guess and tried
>> running without the UNION ALL, which appeared to work correctly.
> Yes, it works fine without UNION ALL.

I took a look at this and it appears to be a bug in the UNION ALL
handling of queries with security quals. An even simpler test case
that triggers it is:

create role test_user1;
drop table if exists foo cascade;
create table foo(a int);
grant all on foo to test_user1;

alter table foo enable row level security;
create policy foo_policy on foo for select using (a > 0);

set role test_user1;
explain (verbose, costs off)

What's happening is that flatten_simple_union_all() and
pull_up_union_leaf_queries() is building translated_vars lists on
appendrels, prior to the security quals being expanded, and those
security quals are adjusted to point to the parent rel. Then the code
to expand the security quals on the child RTEs no longer sees any Vars
pointing to those RTEs, so the resulting subquery RTEs end up with
empty targetlists.

This appears to be a simple oversight in expand_security_qual() -- it
needs to look at and update the Vars in the translated_vars lists of
the appendrels to work properly. I think I wasn't expecting for things
outside the Query to be pointing into its guts in this way.

Attached is a simple patch that appears to work, but it needs more
testing (and some regression tests).

diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
new file mode 100644
index c4b61df..c0d8a35
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -263,7 +263,8 @@ expand_security_qual(PlannerInfo *root,
 			 * 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
-			 * as we go.
+			 * as we go.  Also replace any references in the translated_vars
+			 * lists of any appendrels.
 			context.rt_index = rt_index;
 			context.sublevels_up = 0;
@@ -274,6 +275,8 @@ expand_security_qual(PlannerInfo *root,
 			security_barrier_replace_vars((Node *) parse, &context);
 			security_barrier_replace_vars((Node *) tlist, &context);
+			security_barrier_replace_vars((Node *) root->append_rel_list,
+										  &context);
 			heap_close(context.rel, NoLock);
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to