Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options

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)

In response to

Responses

Browse pgsql-hackers by date

  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().