Subject: Re: pgsql: Apply RLS policies to partitioned tables.
References: <E1dK59q-0005XI-2U@gemulon.postgresql.org>
 <10974.1497227727@sss.pgh.pa.us>
 <0c5b8b18-262d-de09-50e1-6cf4815d74ab@joeconway.com>
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Message-ID: <bea1642d-2791-fe5f-e9cf-df12ea5cf162@lab.ntt.co.jp>
Date: Mon, 12 Jun 2017 11:26:24 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <0c5b8b18-262d-de09-50e1-6cf4815d74ab@joeconway.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: Joe Conway <mail@joeconway.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@postgresql.org
Precedence: bulk
Sender: pgsql-committers-owner@postgresql.org

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