I revisited the thread:
https://www.postgresql.org/message-id/flat/CAGMVOdsbtRwE_4%2Bv8zjH1d9xfovDeQAGLkP_B6k69_VoFEgX-A%40mail.gmail.com
and came up with attached POC patch (I used some varibale names
appearing in the Krasiyan Andreev's patch). I really love to have
RESPECT/IGNORE NULLS because I believe they are convenient for
users. For FIRST/LAST I am not so excited since there are alternatives
as our document stats, so FIRST/LAST are not included in the patch.
Currently in the patch only nth_value is allowed to use RESPECT/IGNORE
NULLS. I think it's not hard to implement it for others (lead, lag,
first_value and last_value). No document nor test patches are
included for now.
I've actually recently been looking at this feature again recently as well. One thing I wondered, but would need consensus, is to take the SEEK_HEAD/SEEK_TAIL case statements out of WinGetFuncArgInPartition. This function is only called by leadlag_common, which uses SEEK_CURRENT, so those case statements are never reached. Taking them out simplifies the code as it is but means future features might need it re-added (although I'm not sure the use case for it, as that function is for window funcs that ignore the frame options).
Note that RESPECT/IGNORE are not registered as reserved keywords in
this patch (but registered as unreserved keywords). I am not sure if
this is acceptable or not.
> The questions of how we interface to the individual window functions
> are really independent of how we handle the parsing problem. My
> first inclination is to just pass the flags down to the window functions
> (store them in WindowObject and provide some additional inquiry functions
> in windowapi.h) and let them deal with it.
I agree with this. Also I do not change the prototype of
nth_value. So I pass RESPECT/IGNORE NULLS information from the raw
parser to parse/analysis and finally to WindowObject.
This is a much better option than my older patch which needed to change the functions.
> It's also worth wondering if we couldn't just implement the flags in
> some generic fashion and not need to involve the window functions at
> all. FROM LAST, for example, could and perhaps should be implemented
> by inverting the sort order. Possibly IGNORE NULLS could be implemented
> inside the WinGetFuncArgXXX functions? These behaviors might or might
> not make much sense with other window functions, but that doesn't seem
> like it's our problem.
Yes, probably we could make WinGetFuncArgXXX a little bit smarter in
this direction (not implemented in the patch at this point).
+1 for doing it here. Maybe also refactor WinGetFuncArgInFrame, putting the exclusion checks in a static function as that function is already pretty big?
