On Tue, Jun 18, 2013 at 6:27 PM, Nicholas White <n.j.wh...@gmail.com> wrote: > Thanks for the reviews. I've attached a revised patch that has the lexer > refactoring Alvaro mentions (arbitarily using a goto rather than a bool > flag) and uses Jeff's idea of just storing the index of the last non-null > value (if there is one) in the window function's context (and not the Datum > itself). > > However, Robert's right that SELECT ... ORDER BY respect NULLS LAST will now > fail. An earlier iteration of the patch had RESPECT and IGNORE as reserved, > but that would have broken tables with columns called "respect" (etc.), > which the current version allows. Is this backwards incompatibility > acceptable?
I think it's better to add new partially reserved keywords than to have this kind of random breakage. When you make something a partially-reserved keyword, then things break, but in a fairly well-defined way. Lexer hacks can break things in ways that are much subtler, which we may not even realize for a long time, and which in that case would mean that the word "respect" needs to be quoted in some contexts but not others. That's going to cause a lot of headaches, because pg_dump etc. know about quoting reserved keywords, but they don't know anything about quoting unreserved keywords in contexts where they might happen to be followed by the wrong next word. > If not, maybe I should try doing a two-token lookahead in the > token-aggregating code between the lexer and the parser (i.e. make a > RESPECT_NULLS token out of a sequence of RESPECT NULLS OVER tokens, > remembering to replace the OVER token)? Or what about adding an %expect > statement to the Bison grammar, confirming that the shift / reduce conflicts > caused by using the RESPECT, IGNORE & NULL_P tokens the in out_clause rule > are OK? These lines of inquiry don't seem promising to me. It's going to be complicated and unmaintainable and may just move the failure scenarios to cases that are too obscure for us to reason about. I think the question is whether this feature is really worth adding new reserved keywords for. I have a hard time saying we shouldn't support something that's part of the SQL standard, but personally, it's not something I've seen come up prior to this thread. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
