Horton

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

Vik Fearing <vik@postgresfriends.org>

View thread
From: Vik Fearing <vik@postgresfriends.org>
To: Tatsuo Ishii <ishii@sraoss.co.jp>, tgl@sss.pgh.pa.us
Cc: andrew@tao11.riddles.org.uk, pgsql-hackers@postgresql.org, david@fetter.org, ojford@gmail.com, krasiyan@gmail.com
Date: 2023-04-22T17:27:06Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix coding style with "else".

  2. Fix multi WinGetFuncArgInFrame/Partition calls with IGNORE NULLS.

  3. Fix Coverity issue reported in commit 2273fa32bce.

  4. Use ereport rather than elog in WinCheckAndInitializeNullTreatment.

  5. Avoid uninitialized-variable warnings from older compilers.

  6. Fix Coverity issues reported in commit 25a30bbd423.

  7. Improve EXPLAIN's display of window functions.

  8. Automatically generate node support functions

On 4/22/23 14:14, Tatsuo Ishii 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.

Excellent.  I was thinking about picking my version of this patch up 
again, but I think this might be better than mine.

I am curious why set_mark is false in the IGNORE version instead of also 
being const_offset.  Surely the nth non-null in the frame will never go 
backwards.

Dealing with marks was the main reason (I think) that my patch was not 
accepted.

> For FIRST/LAST I am not so excited since there are alternatives
> as our document stats,

I disagree with this.  The point of having FROM LAST is to avoid 
calculating a new window and running a new pass over it.

 > so FIRST/LAST are not included in the patch.

I do agree that we can have <null treatment> without <from first or 
last> so let's move forward with this and handle the latter later.

> Currently in the patch only nth_value is allowed to use RESPECT/IGNORE
> NULLS.

This should not be hard coded.  It should be a new field in pg_proc 
(with a sanity check that it is only true for window functions).  That 
way custom window functions can implement it.

> I think it's not hard to implement it for others (lead, lag,
> first_value and last_value).

It doesn't seem like it should be, no.

> No document nor test patches are included for now.

I can volunteer to work on these if you want.

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

For me, this is perfectly okay.  Keep them at the lowest level of 
reservation as possible.
-- 
Vik Fearing