On Sat, 22 Apr 2023, 13:14 Tatsuo Ishii, <is...@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 >