Thread: new keywords in 9.1
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. 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. XMLEXISTS is pretty horrible in that the syntax apparently requires three new keywords (XMLEXISTS, PASSING, REF) which is pretty lame but I guess it's specified by the standard so I'm not sure there's much we can do about it. The rest look reasonable and necessary AFAICT. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
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.
> 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.)
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?
regards, tom lane
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
On 12/03/11 05:18, Robert Haas wrote: > XMLEXISTS is pretty horrible in that the > syntax apparently requires three new keywords (XMLEXISTS, PASSING, > REF) which is pretty lame but I guess it's specified by the standard > so I'm not sure there's much we can do about it. The rest look > reasonable and necessary AFAICT I can confirm that I added the three keywords as described in the SQL/XML standard (section 8.4). Apologies for the delayed confirmation, I missed the thread when it was started and only noticed when your commit message arrived. Regards, -- Mike Fowler Registered Linux user: 379787