Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Nicholas White <n(dot)j(dot)white(at)gmail(dot)com>, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls
Date: 2013-06-17 15:31:22
Message-ID: CA+TgmoZzZq-M=490GKMCvZS0s4ydpaNx6yF+6GfXvt1ksxzsJA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jun 15, 2013 at 9:37 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Nicholas White escribió:
>
>> For the parsing changes, it seems I can either make RESPECT and IGNORE
>> reserved keywords, or add a lookahead to construct synthetic RESPECT NULLS
>> and IGNORE NULLS keywords. The grammar wouldn't compile if RESPECT and
>> IGNORE were just normal unreserved keywords due to ambiguities after a
>> function definition (e.g. select abs(1) respect; - which is currently a
>> valid statement).
>
> Well, making them reserved keywords is not that great, so maybe the
> lookahead thingy is better. However, this patch introduces the third
> and fourth uses of the "save the lookahead token" pattern in the
> "default" switch cases. Can we refactor that bit somehow, to avoid so
> many duplicates? (For a minute I thought that Andrew Gierth's WITH
> ORDINALITY patch would add another one, but it seems not.)

Making things reserved keywords is painful and I don't like it, but
I've started to become skeptical of shifting the problem to the lexer,
too. Sometimes special case logic in the lexer about token combining
can have surprising consequences in other parts of the grammar. For
example, with a lexer hack, what will happen if someone has a column
named RESPECT and does SELECT ... ORDER BY respect NULLS LAST? I
haven't studied the code in detail so maybe it's fine, but it's
something to think about.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-06-17 16:21:41 Re: refresh materialized view concurrently
Previous Message Kevin Grittner 2013-06-17 15:21:13 Re: refresh materialized view concurrently