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

On Sat, 22 Apr 2023, 13:14 Tatsuo Ishii, <ishii@sraoss.co.jp> wrote:

> 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?


> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS LLC
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
>