pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Apply RLS policies to partitioned tables.
8+ messages / 3 participants
[nested] [flat]
* pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-11 15:52 Joe Conway <mail@joeconway.com>
0 siblings, 1 reply; 8+ messages in thread
From: Joe Conway @ 2017-06-11 15:52 UTC (permalink / raw)
To: pgsql-committers
Apply RLS policies to partitioned tables.
The new partitioned table capability added a new relkind, namely
RELKIND_PARTITIONED_TABLE. Update fireRIRrules() to apply RLS
policies on RELKIND_PARTITIONED_TABLE as it does RELKIND_RELATION.
In addition, add RLS regression test coverage for partitioned tables.
Issue raised by Fakhroutdinov Evgenievich and patch by Mike Palmiotto.
Regression test editorializing by me.
Discussion: https://postgr.es/m/flat/20170601065959.1486.69906@wrigleys.postgresql.org
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/4f7a95be2c112bdc8da5f7e46cbb743b8ba4cc21
Modified Files
--------------
src/backend/rewrite/rewriteHandler.c | 3 +-
src/test/regress/expected/rowsecurity.out | 428 ++++++++++++++++++++++++++++++
src/test/regress/sql/rowsecurity.sql | 142 ++++++++++
3 files changed, 572 insertions(+), 1 deletion(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 00:35 Tom Lane <tgl@sss.pgh.pa.us>
parent: Joe Conway <mail@joeconway.com>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2017-06-12 00:35 UTC (permalink / raw)
To: Joe Conway <mail@joeconway.com>; +Cc: pgsql-committers
Joe Conway <mail@joeconway.com> writes:
> Apply RLS policies to partitioned tables.
Buildfarm member skink has grown a "make check" failure with this commit.
==28150== VALGRINDERROR-BEGIN
==28150== Invalid read of size 8
==28150== at 0x39A355: ExecInitModifyTable (nodeModifyTable.c:1862)
==28150== by 0x37F0F8: ExecInitNode (execProcnode.c:168)
==28150== by 0x37C219: InitPlan (execMain.c:1044)
==28150== by 0x37C3AF: standard_ExecutorStart (execMain.c:256)
==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
==28150== by 0x4CDE19: PortalRun (pquery.c:800)
==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
==28150== by 0x44FD9B: BackendStartup (postmaster.c:4003)
==28150== Address 0xbbdbec8 is 8,008 bytes inside a recently re-allocated block of size 8,192 alloc'd
==28150== at 0x4C2AB76: malloc (vg_replace_malloc.c:299)
==28150== by 0x6002D2: AllocSetAlloc (aset.c:760)
==28150== by 0x606EB5: MemoryContextAllocZeroAligned (mcxt.c:791)
==28150== by 0x3832B1: CreateExecutorState (execUtils.c:99)
==28150== by 0x37C2E6: standard_ExecutorStart (execMain.c:186)
==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
==28150== by 0x4CDE19: PortalRun (pquery.c:800)
==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
==28150==
==28150== VALGRINDERROR-END
The cited line is the ExecInitQual call here:
/* varno = node->nominalRelation */
mapped_wcoList = map_partition_varattnos(wcoList,
node->nominalRelation,
partrel, rel);
foreach(ll, mapped_wcoList)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
mtstate->mt_plans[i]);
wcoExprs = lappend(wcoExprs, wcoExpr);
}
First guess is that map_partition_varattnos has forgotten to handle
WithCheckOption.qual. If so, though, and if that's not resulting
in visible misbehavior in the regression tests, then we are missing
a case that the regression tests should be covering.
BTW, it might be advisable to use castNode(WithCheckOption, ...)
in the line before that.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 02:09 Joe Conway <mail@joeconway.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 8+ messages in thread
From: Joe Conway @ 2017-06-12 02:09 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@postgresql.org, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
On 06/11/2017 05:35 PM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Apply RLS policies to partitioned tables.
>
> Buildfarm member skink has grown a "make check" failure with this commit.
>
> ==28150== VALGRINDERROR-BEGIN
> ==28150== Invalid read of size 8
> ==28150== at 0x39A355: ExecInitModifyTable (nodeModifyTable.c:1862)
> ==28150== by 0x37F0F8: ExecInitNode (execProcnode.c:168)
> ==28150== by 0x37C219: InitPlan (execMain.c:1044)
> ==28150== by 0x37C3AF: standard_ExecutorStart (execMain.c:256)
> ==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
> ==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
> ==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
> ==28150== by 0x4CDE19: PortalRun (pquery.c:800)
> ==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
> ==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
> ==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
> ==28150== by 0x44FD9B: BackendStartup (postmaster.c:4003)
> ==28150== Address 0xbbdbec8 is 8,008 bytes inside a recently re-allocated block of size 8,192 alloc'd
> ==28150== at 0x4C2AB76: malloc (vg_replace_malloc.c:299)
> ==28150== by 0x6002D2: AllocSetAlloc (aset.c:760)
> ==28150== by 0x606EB5: MemoryContextAllocZeroAligned (mcxt.c:791)
> ==28150== by 0x3832B1: CreateExecutorState (execUtils.c:99)
> ==28150== by 0x37C2E6: standard_ExecutorStart (execMain.c:186)
> ==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
> ==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
> ==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
> ==28150== by 0x4CDE19: PortalRun (pquery.c:800)
> ==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
> ==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
> ==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
> ==28150==
> ==28150== VALGRINDERROR-END
>
> The cited line is the ExecInitQual call here:
>
> /* varno = node->nominalRelation */
> mapped_wcoList = map_partition_varattnos(wcoList,
> node->nominalRelation,
> partrel, rel);
> foreach(ll, mapped_wcoList)
> {
> WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
> ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
> mtstate->mt_plans[i]);
>
> wcoExprs = lappend(wcoExprs, wcoExpr);
> }
>
> First guess is that map_partition_varattnos has forgotten to handle
> WithCheckOption.qual. If so, though, and if that's not resulting
> in visible misbehavior in the regression tests, then we are missing
> a case that the regression tests should be covering.
>
> BTW, it might be advisable to use castNode(WithCheckOption, ...)
> in the line before that.
Drat. I'll take a look, but it would probably be good if someone
generally familiar with the partitioned tables patches have a look as well.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
Attachments:
[application/pgp-signature] signature.asc (818B, ../../0c5b8b18-262d-de09-50e1-6cf4815d74ab@joeconway.com/2-signature.asc)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 02:26 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
parent: Joe Conway <mail@joeconway.com>
0 siblings, 1 reply; 8+ messages in thread
From: Amit Langote @ 2017-06-12 02:26 UTC (permalink / raw)
To: Joe Conway <mail@joeconway.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers
On 2017/06/12 11:09, Joe Conway wrote:
> On 06/11/2017 05:35 PM, Tom Lane wrote:
>> Joe Conway <mail@joeconway.com> writes:
>>> Apply RLS policies to partitioned tables.
>>
>> Buildfarm member skink has grown a "make check" failure with this commit.
>>
>> ==28150== VALGRINDERROR-BEGIN
>> ==28150== Invalid read of size 8
>> ==28150== at 0x39A355: ExecInitModifyTable (nodeModifyTable.c:1862)
>> ==28150== by 0x37F0F8: ExecInitNode (execProcnode.c:168)
>> ==28150== by 0x37C219: InitPlan (execMain.c:1044)
>> ==28150== by 0x37C3AF: standard_ExecutorStart (execMain.c:256)
>> ==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
>> ==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
>> ==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
>> ==28150== by 0x4CDE19: PortalRun (pquery.c:800)
>> ==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
>> ==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
>> ==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
>> ==28150== by 0x44FD9B: BackendStartup (postmaster.c:4003)
>> ==28150== Address 0xbbdbec8 is 8,008 bytes inside a recently re-allocated block of size 8,192 alloc'd
>> ==28150== at 0x4C2AB76: malloc (vg_replace_malloc.c:299)
>> ==28150== by 0x6002D2: AllocSetAlloc (aset.c:760)
>> ==28150== by 0x606EB5: MemoryContextAllocZeroAligned (mcxt.c:791)
>> ==28150== by 0x3832B1: CreateExecutorState (execUtils.c:99)
>> ==28150== by 0x37C2E6: standard_ExecutorStart (execMain.c:186)
>> ==28150== by 0x37C4B8: ExecutorStart (execMain.c:151)
>> ==28150== by 0x4CCCCE: ProcessQuery (pquery.c:157)
>> ==28150== by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
>> ==28150== by 0x4CDE19: PortalRun (pquery.c:800)
>> ==28150== by 0x4C9E85: exec_simple_query (postgres.c:1099)
>> ==28150== by 0x4CBF20: PostgresMain (postgres.c:4087)
>> ==28150== by 0x44DC04: BackendRun (postmaster.c:4331)
>> ==28150==
>> ==28150== VALGRINDERROR-END
>>
>> The cited line is the ExecInitQual call here:
>>
>> /* varno = node->nominalRelation */
>> mapped_wcoList = map_partition_varattnos(wcoList,
>> node->nominalRelation,
>> partrel, rel);
>> foreach(ll, mapped_wcoList)
>> {
>> WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
>> ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
>> mtstate->mt_plans[i]);
>>
>> wcoExprs = lappend(wcoExprs, wcoExpr);
>> }
>>
>> First guess is that map_partition_varattnos has forgotten to handle
>> WithCheckOption.qual. If so, though, and if that's not resulting
>> in visible misbehavior in the regression tests, then we are missing
>> a case that the regression tests should be covering.
>>
>> BTW, it might be advisable to use castNode(WithCheckOption, ...)
>> in the line before that.
>
> Drat. I'll take a look, but it would probably be good if someone
> generally familiar with the partitioned tables patches have a look as well.
I am looking too. Commit 587cda35ca3 which added that code did add a test
in updatable_views.sql, but tests added by this commit have perhaps
exposed something not previously covered.
Thanks,
Amit
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 02:51 Tom Lane <tgl@sss.pgh.pa.us>
parent: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2017-06-12 02:51 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Joe Conway <mail@joeconway.com>; pgsql-committers
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> On 2017/06/12 11:09, Joe Conway wrote:
>> On 06/11/2017 05:35 PM, Tom Lane wrote:
>>> First guess is that map_partition_varattnos has forgotten to handle
>>> WithCheckOption.qual.
>> Drat. I'll take a look, but it would probably be good if someone
>> generally familiar with the partitioned tables patches have a look as well.
> I am looking too. Commit 587cda35ca3 which added that code did add a test
> in updatable_views.sql, but tests added by this commit have perhaps
> exposed something not previously covered.
My first guess above is wrong -- the undefined reference is actually
"mtstate->mt_plans[i]". This is because mtstate->mt_num_partitions is
more than mtstate->mt_nplans in the failing case, so that the blithe
assumption that we can use the partition number to index into
mtstate->mt_plans is certainly wrong. Don't know how this should
be corrected, offhand, but do we really have more than one plan
for a partitioned query?
The specific query that's failing is
(gdb) p debug_query_string
$1 = 0x11b6220 "INSERT INTO part_document VALUES (100, 11, 5, 'regress_rls_dave', 'testing pp1');"
and you can get a crash without all the valgrind effort by adding
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index bf26488..5013a6d 100644
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
*************** ExecInitModifyTable(ModifyTable *node, E
*** 1844,1849 ****
--- 1844,1850 ----
Assert(operation == CMD_INSERT);
resultRelInfo = mtstate->mt_partitions;
+ Assert(mtstate->mt_num_partitions == mtstate->mt_nplans);
wcoList = linitial(node->withCheckOptionLists);
for (i = 0; i < mtstate->mt_num_partitions; i++)
{
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 06:48 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 8+ messages in thread
From: Amit Langote @ 2017-06-12 06:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Joe Conway <mail@joeconway.com>; pgsql-committers
On 2017/06/12 11:51, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> On 2017/06/12 11:09, Joe Conway wrote:
>>> On 06/11/2017 05:35 PM, Tom Lane wrote:
>>>> First guess is that map_partition_varattnos has forgotten to handle
>>>> WithCheckOption.qual.
>
>>> Drat. I'll take a look, but it would probably be good if someone
>>> generally familiar with the partitioned tables patches have a look as well.
>
>> I am looking too. Commit 587cda35ca3 which added that code did add a test
>> in updatable_views.sql, but tests added by this commit have perhaps
>> exposed something not previously covered.
>
> My first guess above is wrong -- the undefined reference is actually
> "mtstate->mt_plans[i]". This is because mtstate->mt_num_partitions is
> more than mtstate->mt_nplans in the failing case, so that the blithe
> assumption that we can use the partition number to index into
> mtstate->mt_plans is certainly wrong.
Yes, the code should never access mtstate->mt_plans[i] for i > 0, because
in this case (the INSERT case), there is only one plan. Unlike
UPDATE/DELETE cases, we don't build plans for every partition in the
INSERT case.
> Don't know how this should
> be corrected, offhand, but do we really have more than one plan
> for a partitioned query?
There can't be, as mentioned, in the INSERT case.
How about something like the attached? The patch makes
mtstate->mt_plans[0] to be passed for all invocations of ExecInitQual() on
WithCheckOption.qual that is being initialized for each partition. Note
that we are passing a PlanState that's based on the root parent table's
properties to initialize WITH CHECK OPTION quals to be checked against
rows that will be inserted into partitions (that is, after tuple-routing).
I tried to come up with a test that would cause the existing (unpatched)
code to crash, but couldn't. How would one define a policy or WITH CHECK
OPTIONS view or something else, such that ExecInitQual() called on
WithCheckOptions.qual would try to access parent planstate and would crash
once parent becomes an invalid pointer (mtstate->mt_plans[i] for i > 0)?
After looking at the code downstream to ExecInitQual(), it seems that the
following cases would cases would cause parent planstate to be accessed:
* Qual references a whole row var
* Qual references an Aggref or a GroupingFunc or a WindowFunc
* Qual references a (Alternative) SubPlan
Thanks,
Amit
From 90fdad27c04db70f78c24e4999cd3aef7c06d1f7 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Mon, 12 Jun 2017 14:35:05 +0900
Subject: [PATCH] Don't pass invalid PlanState pointer to ExecInitQual
Current code in ExecInitModifyTable that initializes
WithCheckOption.qual failed to consider that there *isn't* one
plan per partition in the ModifyTableState.mt_plans[] array.
---
src/backend/executor/nodeModifyTable.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index bf26488c51..89991e34ae 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1841,8 +1841,17 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->withCheckOptionLists != NIL && mtstate->mt_num_partitions > 0)
{
List *wcoList;
+ PlanState *plan;
- Assert(operation == CMD_INSERT);
+ /*
+ * In case of INSERT on partitioned tables, there is only plan; in the
+ * planner we don't build one for each partition. Likewise, there is
+ * only one WITH CHECK OPTIONS list, not one per partition.
+ */
+ Assert(operation == CMD_INSERT &&
+ list_length(node->withCheckOptionLists) == 1 &&
+ mtstate->mt_nplans == 1);
+ plan = mtstate->mt_plans[0]; /* The only plan */
resultRelInfo = mtstate->mt_partitions;
wcoList = linitial(node->withCheckOptionLists);
for (i = 0; i < mtstate->mt_num_partitions; i++)
@@ -1858,10 +1867,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
partrel, rel);
foreach(ll, mapped_wcoList)
{
- WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
- ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
- mtstate->mt_plans[i]);
+ WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
+ ExprState *wcoExpr = ExecInitQual((List *) wco->qual, plan);
wcoExprs = lappend(wcoExprs, wcoExpr);
}
--
2.11.0
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Attachments:
[text/plain] 0001-Don-t-pass-invalid-PlanState-pointer-to-ExecInitQual.patch (2.0K, ../../89e6ae66-96ee-b6eb-7195-d88dbb988f2f@lab.ntt.co.jp/2-0001-Don-t-pass-invalid-PlanState-pointer-to-ExecInitQual.patch)
download | inline diff:
From 90fdad27c04db70f78c24e4999cd3aef7c06d1f7 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Mon, 12 Jun 2017 14:35:05 +0900
Subject: [PATCH] Don't pass invalid PlanState pointer to ExecInitQual
Current code in ExecInitModifyTable that initializes
WithCheckOption.qual failed to consider that there *isn't* one
plan per partition in the ModifyTableState.mt_plans[] array.
---
src/backend/executor/nodeModifyTable.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index bf26488c51..89991e34ae 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1841,8 +1841,17 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->withCheckOptionLists != NIL && mtstate->mt_num_partitions > 0)
{
List *wcoList;
+ PlanState *plan;
- Assert(operation == CMD_INSERT);
+ /*
+ * In case of INSERT on partitioned tables, there is only plan; in the
+ * planner we don't build one for each partition. Likewise, there is
+ * only one WITH CHECK OPTIONS list, not one per partition.
+ */
+ Assert(operation == CMD_INSERT &&
+ list_length(node->withCheckOptionLists) == 1 &&
+ mtstate->mt_nplans == 1);
+ plan = mtstate->mt_plans[0]; /* The only plan */
resultRelInfo = mtstate->mt_partitions;
wcoList = linitial(node->withCheckOptionLists);
for (i = 0; i < mtstate->mt_num_partitions; i++)
@@ -1858,10 +1867,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
partrel, rel);
foreach(ll, mapped_wcoList)
{
- WithCheckOption *wco = (WithCheckOption *) lfirst(ll);
- ExprState *wcoExpr = ExecInitQual((List *) wco->qual,
- mtstate->mt_plans[i]);
+ WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
+ ExprState *wcoExpr = ExecInitQual((List *) wco->qual, plan);
wcoExprs = lappend(wcoExprs, wcoExpr);
}
--
2.11.0
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-12 13:46 Tom Lane <tgl@sss.pgh.pa.us>
parent: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2017-06-12 13:46 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Joe Conway <mail@joeconway.com>; pgsql-committers
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> How about something like the attached? The patch makes
> mtstate->mt_plans[0] to be passed for all invocations of ExecInitQual() on
> WithCheckOption.qual that is being initialized for each partition.
Well, the question then is whether that behaves correctly. Where it would
matter is if the WCO qual contains a SubPlan, because you'd be adding
multiple instances of the subplan to the PlanState's list. I think it's
probably all right, but it might be wise to add a test case where there
is such a subplan.
Spacing and comment wording seem a bit random in this, too.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pgsql: Apply RLS policies to partitioned tables.
@ 2017-06-13 04:20 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 8+ messages in thread
From: Amit Langote @ 2017-06-13 04:20 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Joe Conway <mail@joeconway.com>; pgsql-committers
On 2017/06/12 22:46, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> How about something like the attached? The patch makes
>> mtstate->mt_plans[0] to be passed for all invocations of ExecInitQual() on
>> WithCheckOption.qual that is being initialized for each partition.
>
> Well, the question then is whether that behaves correctly. Where it would
> matter is if the WCO qual contains a SubPlan, because you'd be adding
> multiple instances of the subplan to the PlanState's list. I think it's
> probably all right, but it might be wise to add a test case where there
> is such a subplan.
>
> Spacing and comment wording seem a bit random in this, too.
Was about to send the updated patch, but noticed that you already
committed it after adding the test. Thanks a lot.
Regards,
Amit
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2017-06-13 04:20 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-06-11 15:52 pgsql: Apply RLS policies to partitioned tables. Joe Conway <mail@joeconway.com>
2017-06-12 00:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-12 02:09 ` Joe Conway <mail@joeconway.com>
2017-06-12 02:26 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2017-06-12 02:51 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-12 06:48 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2017-06-12 13:46 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-06-13 04:20 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox