From: Tom Lane <tgl@sss.pgh.pa.us>
To: Joe Conway <mail@joeconway.com>
cc: pgsql-committers@postgresql.org
Subject: Re: pgsql: Apply RLS policies to partitioned tables.
In-reply-to: <E1dK59q-0005XI-2U@gemulon.postgresql.org>
References: <E1dK59q-0005XI-2U@gemulon.postgresql.org>
Comments: In-reply-to Joe Conway <mail@joeconway.com>
	message dated "Sun, 11 Jun 2017 15:52:06 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <10973.1497227727.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 11 Jun 2017 20:35:27 -0400
Message-ID: <10974.1497227727@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-committers-owner@postgresql.org

Joe Conway <mail@joeconway.com> writes:
> Apply RLS policies to partitioned tables.

Buildfarm member skink has grown a "make check" failure with this commit.

=3D=3D28150=3D=3D VALGRINDERROR-BEGIN
=3D=3D28150=3D=3D Invalid read of size 8
=3D=3D28150=3D=3D    at 0x39A355: ExecInitModifyTable (nodeModifyTable.c:18=
62)
=3D=3D28150=3D=3D    by 0x37F0F8: ExecInitNode (execProcnode.c:168)
=3D=3D28150=3D=3D    by 0x37C219: InitPlan (execMain.c:1044)
=3D=3D28150=3D=3D    by 0x37C3AF: standard_ExecutorStart (execMain.c:256)
=3D=3D28150=3D=3D    by 0x37C4B8: ExecutorStart (execMain.c:151)
=3D=3D28150=3D=3D    by 0x4CCCCE: ProcessQuery (pquery.c:157)
=3D=3D28150=3D=3D    by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
=3D=3D28150=3D=3D    by 0x4CDE19: PortalRun (pquery.c:800)
=3D=3D28150=3D=3D    by 0x4C9E85: exec_simple_query (postgres.c:1099)
=3D=3D28150=3D=3D    by 0x4CBF20: PostgresMain (postgres.c:4087)
=3D=3D28150=3D=3D    by 0x44DC04: BackendRun (postmaster.c:4331)
=3D=3D28150=3D=3D    by 0x44FD9B: BackendStartup (postmaster.c:4003)
=3D=3D28150=3D=3D  Address 0xbbdbec8 is 8,008 bytes inside a recently re-al=
located block of size 8,192 alloc'd
=3D=3D28150=3D=3D    at 0x4C2AB76: malloc (vg_replace_malloc.c:299)
=3D=3D28150=3D=3D    by 0x6002D2: AllocSetAlloc (aset.c:760)
=3D=3D28150=3D=3D    by 0x606EB5: MemoryContextAllocZeroAligned (mcxt.c:791)
=3D=3D28150=3D=3D    by 0x3832B1: CreateExecutorState (execUtils.c:99)
=3D=3D28150=3D=3D    by 0x37C2E6: standard_ExecutorStart (execMain.c:186)
=3D=3D28150=3D=3D    by 0x37C4B8: ExecutorStart (execMain.c:151)
=3D=3D28150=3D=3D    by 0x4CCCCE: ProcessQuery (pquery.c:157)
=3D=3D28150=3D=3D    by 0x4CCEDF: PortalRunMulti (pquery.c:1287)
=3D=3D28150=3D=3D    by 0x4CDE19: PortalRun (pquery.c:800)
=3D=3D28150=3D=3D    by 0x4C9E85: exec_simple_query (postgres.c:1099)
=3D=3D28150=3D=3D    by 0x4CBF20: PostgresMain (postgres.c:4087)
=3D=3D28150=3D=3D    by 0x44DC04: BackendRun (postmaster.c:4331)
=3D=3D28150=3D=3D=20
=3D=3D28150=3D=3D VALGRINDERROR-END

The cited line is the ExecInitQual call here:

            /* varno =3D node->nominalRelation */
            mapped_wcoList =3D map_partition_varattnos(wcoList,
                                                     node->nominalRelation,
                                                     partrel, rel);
            foreach(ll, mapped_wcoList)
            {
                WithCheckOption *wco =3D (WithCheckOption *) lfirst(ll);
                ExprState  *wcoExpr =3D ExecInitQual((List *) wco->qual,
                                                   mtstate->mt_plans[i]);

                wcoExprs =3D 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


--=20
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers