Subject: Re: pgsql: Apply RLS policies to partitioned tables.
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@postgresql.org,
 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
References: <E1dK59q-0005XI-2U@gemulon.postgresql.org>
 <10974.1497227727@sss.pgh.pa.us>
From: Joe Conway <mail@joeconway.com>
Openpgp: id=4EA5B4659ED59CC9538139A5332B7E68B699434A
Message-ID: <0c5b8b18-262d-de09-50e1-6cf4815d74ab@joeconway.com>
Date: Sun, 11 Jun 2017 19:09:26 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <10974.1497227727@sss.pgh.pa.us>
Content-Type: multipart/signed; micalg=pgp-sha256;
 protocol="application/pgp-signature";
 boundary="MmLoppO1uj62s3p2XVDvewDWphaF3aJsu"
Precedence: bulk
Sender: pgsql-committers-owner@postgresql.org

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--MmLoppO1uj62s3p2XVDvewDWphaF3aJsu
From: Joe Conway <mail@joeconway.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@postgresql.org,
 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Message-ID: <0c5b8b18-262d-de09-50e1-6cf4815d74ab@joeconway.com>
Subject: Re: pgsql: Apply RLS policies to partitioned tables.
References: <E1dK59q-0005XI-2U@gemulon.postgresql.org>
 <10974.1497227727@sss.pgh.pa.us>
In-Reply-To: <10974.1497227727@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

On 06/11/2017 05:35 PM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Apply RLS policies to partitioned tables.
>=20
> Buildfarm member skink has grown a "make check" failure with this commit.
>=20
> =3D=3D28150=3D=3D VALGRINDERROR-BEGIN
> =3D=3D28150=3D=3D Invalid read of size 8
> =3D=3D28150=3D=3D    at 0x39A355: ExecInitModifyTable (nodeModifyTable.c:=
1862)
> =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-=
allocated 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:7=
91)
> =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
>=20
> The cited line is the ExecInitQual call here:
>=20
>             /* varno =3D node->nominalRelation */
>             mapped_wcoList =3D map_partition_varattnos(wcoList,
>                                                      node->nominalRelatio=
n,
>                                                      partrel, rel);
>             foreach(ll, mapped_wcoList)
>             {
>                 WithCheckOption *wco =3D (WithCheckOption *) lfirst(ll);
>                 ExprState  *wcoExpr =3D ExecInitQual((List *) wco->qual,
>                                                    mtstate->mt_plans[i]);
>=20
>                 wcoExprs =3D lappend(wcoExprs, wcoExpr);
>             }
>=20
> 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.
>=20
> 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


--=20
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


--MmLoppO1uj62s3p2XVDvewDWphaF3aJsu
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJZPffWAAoJEDfy90M199hlYtYQAJFAfW0HBdc7c6iogpN7wgtz
beaZV2KtMxaaONUtdG0JD1/XjiFIbvKElKNZY6pLMLOAvXZfqZy1BF6mmCrHwscC
MEXUEHQZAKMzeyQK+/Hf4m3RqEqSArlD0ijlWSBpicOKNGjPdpYXNM7D9Zgofqaj
COgrITDX5P7xpR/EkDgxh+thpBAIlqRNBbXKcezcWFz3Fwat2LltSH+YYU48ZrgD
+Y2aFArX3Ud1PfhQ0ICBzufwG0AR7UMSGoiuS7FjzJROJcI9dkcLt/3Yo92b4j3q
2QKfyxquwO4ltR973edfjyEJoniZXVy4H+ou6q++tEFp5TaSvSscVOZw9gXtYAVf
lane816qqrtsw0kzjUnoLSxR/gNHWvnBa3D//4Vumwy5uofrqnfWV02tRdMxSzf1
57Tk7glriUXP48l9O3JOH+pJ8TePoWwvj0zfMHRl+xDCkyxWt1fe1mmfy4OKOprK
6+aW5fbGl7b+uWtN2ty6J7bK14ztG2bqz5RgI8LpglkXPC9XzLnA1QUMKh1ryXZj
zHv4xxzeU4zFURSkL16Gtap19JOyXHwb9vxTcIaeDO14WEweLNsKF7WhmJFgR4R5
/eZLD3MJeWU7ZYLevyfM+Zo0FSQD/aZxz02HQg5bYCOE1DWX5vpVcznrnjUrqjEa
TeX0FTVV6NxYaccvO79y
=E8Bo
-----END PGP SIGNATURE-----

--MmLoppO1uj62s3p2XVDvewDWphaF3aJsu--