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

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Nicholas White <n(dot)j(dot)white(at)gmail(dot)com>
Cc: Troels Nielsen <bn(dot)troels(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(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-29 17:30:43
Message-ID: 1372527043.19747.7.camel@jdavis
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Mon, 2013-06-24 at 18:01 +0100, Nicholas White wrote:
> Good catch - I've attached a patch to address your point 1. It now
> returns the below (i.e. correctly doesn't fill in the saved value if
> the index is out of the window. However, I'm not sure whether (e.g.)
> lead-2-ignore-nulls means count forwards two rows, and if that's null
> use the last one you've seen (the current implementation) or count
> forwards two non-null rows (as you suggest). The behaviour isn't
> specified in a (free) draft of the 2003 standard
> (http://www.wiscorp.com/sql_2003_standard.zip), and I don't have
> access to the (non-free) final version. Could someone who does have
> access to it clarify this? I've also added your example to the
> regression test cases.

Reading a later version of the draft, it is specified, but is still
slightly unclear.

As I see it, the standard describes the behavior in terms of eliminating
the NULL rows entirely before applying the offset. This matches Troels's
interpretation. Are you aware of any implementations that do something
different?

> I didn't include this functionality for the first / last value window
> functions as their implementation is currently a bit different; they
> just call WinGetFuncArgInFrame to pick out a single value. Making
> these functions respect nulls would involve changing the single lookup
> to a walk through the tuples to find the first non-null version, and
> keeping track of this index in a struct in the context. As this change
> is reasonably orthogonal I was going to submit it as a separate patch.

Sounds good.

Regards,
Jeff Davis

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2013-06-29 17:54:53 Re: Bugfix and new feature for PGXS
Previous Message Heikki Linnakangas 2013-06-29 17:08:38 Re: GIN improvements part 1: additional information