Re: new keywords in 9.1

On Sat, Mar 12, 2011 at 1:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> It looks like 9.1 currently introduces 11 new keywords: ATTRIBUTE,
>> COLLATION, EXTENSION, LABEL, NOREPLICATION, PASSING, REF, REPLICATION,
>> UNLOGGED, VALIDATE, and XMLEXISTS.  Aside from VALIDATE, which is
>> already being discussed on another thread, are there any of these that
>> we can/should try to get rid of?  At a quick glance, it looks quite
>> simple to avoid making REPLICATION/NOREPLICATION into keywords, and we
>> can actually *remove* a bunch of existing keywords using the same
>> trick.  Patch attached.
>
> One trouble with this trick is that it cannot distinguish between, eg,
> SUPERUSER and "superuser" (with the quotes), whereas the latter really
> ought not act like a keyword.  I'm not sure this is a big enough problem
> to justify bloating the parser with extra keywords, though.

I don't think so.  We are loosey-goosey about that in other places as
well, e.g. you can do EXPLAIN ("analyze") SELECT ....

>> It would be possible to make CREATE UNLOGGED TABLE work without making
>> UNLOGGED a keyword using a similar trick, though it's a bit messy.
>> SELECT .. INTO UNLOGGED foo can't work unless it's a keyword, though,
>> I think, though I wouldn't cry much if we lost that option.  I'm
>> inclined to think this is not worth messing with more on grounds of
>> ugliness than anything else.
>
> -1 for changing that; I think that anything that is used in more than a
> very circumscribed context is likely to come back to bite us if we play
> cute-looking parser tricks.
>
> A minor stylistic gripe:
>
>> +                                     if (!strcmp($1, "superuser"))
>
> Please spell that as
>
> +                                       if (strcmp($1, "superuser") == 0)
>
> which is the house style around here.  (I have a rant on file in the
> archives about exactly why to do that, if you care, but the gist is that
> the former way looks like a not-match rather than a match test.)

Well, I think the fact that it is a house style is relevant, but the
reasons are not.  To me !strcmp is an idiom that I've seen enough
times that I immediately know what it means, whereas the == 0 style
forces me to stop and scratch my head for a minute to figure out what
the sense of the test is.

> One other thought about this code is that you could possibly avoid
> having gram.y bother with knowledge of the specific keywords at all:
> just fling any IDENT into a makeDefElem and let dbcommands.c sort it
> out.  The syntax error messages might get a bit worse (no error pointer,
> in particular) but how much do we care?

I can't see any compelling reason to do more work to make the error
messages worse, and frankly I think it makes more sense to have this
code directly in gram.y.  It's obviously possible to overdo that
theory, but in this case ISTM it reduces the amount of tracing through
code that must be done to understand how it all works, without really
costing anything.

So, committed, with just the stylistic change mentioned above.  Thanks
for the review.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company