From: | Mike Palmiotto <mike(dot)palmiotto(at)crunchydata(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Joe Conway <mail(at)joeconway(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, fte(at)nct(dot)ru, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>, Amit Langote <amitlangote09(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: BUG #14682: row level security not work with partitioned table |
Date: | 2017-06-07 13:49:27 |
Message-ID: | CAMN686F9CS_L70h1sZCRfWW7dKFW3vZMVpqgsQCkuFt=HymUKw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Tue, Jun 6, 2017 at 9:12 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Jun 7, 2017 at 9:52 AM, Joe Conway <mail(at)joeconway(dot)com> wrote:
>> Thanks Mike. I'll take a close look to verify output correctnes, but I
>> am concerned that the new tests are unnecessarily complex. Any other
>> opinions on that?
>
> Some tests would be good to have. Now, if I read those regression
> tests correctly, this is using 10 relations where two would be enough,
> one as the parent relation and one as a partition. Then policies apply
> on the parent relation. The same kind of policy is defined 4 times,
> and there is bloat with GRANT and ALTER TABLE commands.
I ended up narrowing it down to 4 tables (one parent and 3 partitions)
in order to demonstrate policy sorting and order of RLS/partition
constraint checking. It should be much more straight-forward now, but
let me know if there are any further recommended changes.
One thing that concerns me is the first EXPLAIN plan from regress_rls_dave:
+EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle);
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------
+ Append
+ InitPlan 1 (returns $0)
+ -> Index Scan using uaccount_pkey on uaccount
+ Index Cond: (pguser = CURRENT_USER)
+ -> Seq Scan on part_document_fiction
+ Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND
(dlevel <= $0) AND f_leak(dtitle))
+ -> Seq Scan on part_document_satire
+ Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND
(dlevel <= $0) AND f_leak(dtitle))
+(8 rows)
I would expect that both part_document_satire (cid == 55) and
part_document_nonfiction (cid == 99) would be excluded from the
explain, but only cid < 99 seems to work. Interestingly, when I change
policy pp1r to cid < 55, I see the following:
+EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle);
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------
+ Append
+ InitPlan 1 (returns $0)
+ -> Index Scan using uaccount_pkey on uaccount
+ Index Cond: (pguser = CURRENT_USER)
+ -> Seq Scan on part_document_fiction
+ Filter: ((cid < 55) AND (cid <> 55) AND (cid < 99) AND
(dlevel <= $0) AND f_leak(dtitle))
+(6 rows)
Is this a demonstration of a non-immutable function backing the
operator and thus not being able to filter it from the planner, or is
it a bug?
>
> +SELECT * FROM part_document;
> + did | cid | dlevel | dauthor | dtitle
> +-----+-----+--------+-------------------+-------------------------
> + 1 | 11 | 1 | regress_rls_bob | my first novel
> Adding an "ORDER BY did" as well here would make the test output more
> predictable.
Done.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
Attachment | Content-Type | Size |
---|---|---|
0001-Add-RLS-support-to-partitioned-tables.patch | text/x-patch | 34.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2017-06-07 14:36:45 | Re: BUG #14693: create materialized view forces btrim |
Previous Message | Noah Misch | 2017-06-07 07:57:52 | Re: [PATCH] Fixed malformed error message on malformed SCRAM message. |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2017-06-07 13:55:29 | Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken) |
Previous Message | Tom Lane | 2017-06-07 13:24:46 | Re: intermittent failures in Cygwin from select_parallel tests |