From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: pgsql-hackers@postgresql.org
Cc: david@fetter.org,  Oliver Ford <ojford@gmail.com>,
 Krasiyan Andreev <krasiyan@gmail.com>
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
In-Reply-To: 
 <CAN1PwonR8uFAN-cCYRB1eReyNFNSdMtvJFYHWdmp=vCoGxuKdQ@mail.gmail.com>
	(Krasiyan Andreev's message of "Tue, 18 Sep 2018 15:05:07 +0300")
Message-ID: <878t3vj279.fsf@news-spur.riddles.org.uk>
References: 
 <CAGMVOdsbtRwE_4+v8zjH1d9xfovDeQAGLkP_B6k69_VoFEgX-A@mail.gmail.com>
	<20180728185958.GP17411@fetter.org>
	<CAN1PwonR8uFAN-cCYRB1eReyNFNSdMtvJFYHWdmp=vCoGxuKdQ@mail.gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix)
Date: Sat, 22 Sep 2018 21:06:32 +0100
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

>>>>> "Krasiyan" == Krasiyan Andreev <krasiyan@gmail.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)