Date: Sun, 11 Sep 2016 21:57:48 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@2ndquadrant.com>,
	Thom Brown <thom@linux.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Add support for restrictive RLS policies
Message-ID: <20160912015748.GF11453@tamriel.snowman.net>
References: <20160908183544.GU4028@tamriel.snowman.net>
 <20160908192157.GA53172@alvherre.pgsql>
 <20160908200912.GW4028@tamriel.snowman.net>
 <5611.1473369685@sss.pgh.pa.us>
 <CA+Tgmoaf9A5dS7rdoxsyQe6=rxH=nNSV1pfcz-gRd_p8cSiOZQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="1Ow488MNN9B9o/ov"
Content-Disposition: inline
In-Reply-To: 
 <CA+Tgmoaf9A5dS7rdoxsyQe6=rxH=nNSV1pfcz-gRd_p8cSiOZQ@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--1Ow488MNN9B9o/ov
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Thu, Sep 8, 2016 at 5:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Stephen Frost <sfrost@snowman.net> writes:
> >> * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> >>> Can't you keep those words as Sconst or something (DefElems?) until t=
he
> >>> execution phase, so that they don't need to be keywords at all?
> >
> >> Seems like we could do that, though I'm not convinced that it really
> >> gains us all that much.  These are only unreserved keywords, of course,
> >> so they don't impact users the way reserved keywords (of any kind) can.
> >> While there may be some places where we use a string to represent a set
> >> of defined options, I don't believe that's typical
> >
> > -1 for having to write them as string literals; but I think what Alvaro
> > really means is to arrange for the words to just be identifiers in the
> > grammar, which you strcmp against at execution.  See for example
> > reloption_list.  (Whether you use DefElem as the internal representation
> > is a minor detail, though it might help for making the parsetree
> > copyObject-friendly.)
> >
> > vacuum_option_elem shows another way to avoid making a word into a
> > keyword, although to me that one is more of an antipattern; it'd be bet=
ter
> > to leave the strcmp to execution, since there's so much other code that
> > does things that way.
>=20
> There are other cases like that, too, e.g. AlterOptRoleElem; I don't
> think it's a bad pattern.  Regardless of the specifics, I do think
> that it would be better not to bloat the keyword table with things
> that don't really need to be keywords.

The AlterOptRoleElem case is, I believe, what Tom was just suggesting as
an antipattern, since the strcmp() is being done at parse time instead
of at execution time.

If we are concerned about having too many unreserved keywords, then I
agree that AlterOptRoleElem is a good candidate to look at for reducing
the number we have, as it appears to contain 3 keywords which are not
used anywhere else (and 1 other which is only used in one other place).

I do think that using IDENT for the various role attributes does make
sense, to be clear, as there are quite a few of them, they change
depending on major version, and those keywords are very unlikely to ever
have utilization elsewhere.

For this case, there's just 2 keywords which seem like they may be used
again (perhaps for ALTER or DROP POLICY, or default policies if we grow
those in the future), and we're unlikly to grow more in the future for
that particular case (as we only have two binary boolean operators and
that seems unlikely to change), though, should that happens, we could
certainly revisit this.

Thanks!

Stephen

--1Ow488MNN9B9o/ov
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJX1gucAAoJEO1sijiDR2RVA10QALbQR3i1bwZ/AdVRMIRQi/Wr
EtolB20bmrAIu6N73JhQW5Ffb2VjciiZQjSvbogiUargyIiJNnU85MvWiNmE1dco
eCwMRLJ9TG2kYAk4AgDkS3Jjw6UrZRkD5T4yiuEKCFCP/Izt0rw/s1vVUI6IiGpt
fa1FGF5cuthXzne4E0Urg8xVa9EJnLAwk8Wh2zGxQthUxuqJNjZ8qH8Jc2buoPTr
8WKMOCr12N0WRmZpfqiKlWtUMrvAV7vuuVJzrFH2Yr20x2rCjQYiC+QqQRv2EDuC
mFlXNSgIeKwNMwI/SlAyiXVBguP7nmbXjPKbf0+zrr03w7eQ+ovXiBUKjlNWgOoH
pjtESdlkA0SCoOWEkteVtCD6GX2Y7BXoRj3N8iPT/QO326VAu0mhAoCQ2jZCGTtf
zXrD2KdqMcGa6PUnQxwEpBCNiinya3DWVPsk7qAnS4PK1o4Giwhnk7FDkSZ2i2Fu
vAhsxXwzBsyvW9whTAuSXthfT414ZQRuUEuq4q7t2GHg8SqO/YfFCKpuq3kvvRYi
7HTyX+FQmVv9jhlnUtsXMCw6RwUn15OVou2Ifa2VpZ9TWAXmARCLh/Nm05PlN25R
26b655GAqowK+DMk0Tt+jry+2Zqsd+/eqOo74zrm8XjqHOb30jYIi6/mw7RCXpYZ
OxSoqnVwxx4wcZV1/d25
=hKXf
-----END PGP SIGNATURE-----

--1Ow488MNN9B9o/ov--