From: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | david(at)fetter(dot)org, Oliver Ford <ojford(at)gmail(dot)com>, Krasiyan Andreev <krasiyan(at)gmail(dot)com> |
Subject: | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options |
Date: | 2018-09-22 20:06:32 |
Message-ID: | 878t3vj279.fsf@news-spur.riddles.org.uk |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
>>>>> "Krasiyan" == Krasiyan Andreev <krasiyan(at)gmail(dot)com> writes:
Krasiyan> Hi,
Krasiyan> Patch applies and compiles, all included tests and building
Krasiyan> of the docs pass. I am using last version from more than two
Krasiyan> months ago in production environment with real data and I
Krasiyan> didn't find any bugs, so I'm marking this patch as ready for
Krasiyan> committer in the commitfest app.
Unfortunately, reviewing it from a committer perspective - I can't
possibly commit this as it stands, and anything I did to it would be
basically a rewrite of much of it.
Some of the problems could be fixed. For example the type names could be
given pg_* prefixes (it's clearly not acceptable to create random
special-purpose boolean subtypes in pg_catalog and _not_ give them such
a prefix), and the precedence hackery in gram.y could have comments
added (gram.y is already bad enough; _anything_ fancy with precedence
has to be described in the comments). But I don't like that hack with
the special types at all, and I think that needs a better solution.
Normally I'd push hard to try and get some solution that's sufficiently
generic to allow user-defined functions to make use of the feature. But
I think the SQL spec people have managed to make that literally
impossible in this case, what with the FROM keyword appearing in the
middle of a production and not followed by anything sufficiently
distinctive to even use for extra token lookahead.
Also, as has been pointed out in a number of previous features, we're
starting to accumulate identifiers that are reserved in subtly different
ways from our basic four-category system (which is itself a significant
elaboration compared to the spec's simple reserved/unreserved
distinction). As I recall this objection was specifically raised for
CUBE, but justified there by the existence of the contrib/cube extension
(and the fact that the standard CUBE() construct is used only in very
specific places in the syntax). This patch would make lead / lag /
first_value / last_value / nth_value syntactically "special" while not
actually reserving them (beyond having them in unreserved_keywords); I
think serious consideration should be given to whether they should
instead become col_name_keywords (which would, I believe, make it
unnecessary to mess with precedence).
Anyone have any thoughts or comments on the above?
--
Andrew (irc:RhodiumToad)
From | Date | Subject | |
---|---|---|---|
Next Message | Chapman Flack | 2018-09-22 21:56:38 | Re: vary read_only in SPI calls? or poke at the on-entry snapshot? |
Previous Message | Alexander Korotkov | 2018-09-22 19:00:12 | Re: [HACKERS] Bug in to_timestamp(). |