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.
select val, lead(val, 2) ignore nulls over (order by id) from test_table;
val | lead
-----+------
1 | 3
2 | 4
3 | 4
4 | 4
| 4
| 5
| 6
5 | 7
6 |
7 |
(10 rows)
If the other reviewers are happy with your grammar changes then I'll merge them into the patch. Alternatively, if departing from the standard is OK then we could reorder the keywords so that a window function is like SELECT lag(x,1) OVER RESPECT NULLS (ORDER BY y) - i.e. putting the respect / ignore tokens after the OVER reserved keyword. Although non-standard it'd make the grammar change trivial.
> Also, I think someone mentioned this already, but what about
> first_value() and last_value()? Shouldn't we do those at the same time?
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.