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

Oliver Ford <ojford@gmail.com>

From: Oliver Ford <ojford@gmail.com>
To: Tatsuo Ishii <ishii@postgresql.org>
Cc: krasiyan@gmail.com, tgl@sss.pgh.pa.us, vik@postgresfriends.org, pgsql-hackers@postgresql.org, andrew@tao11.riddles.org.uk, david@fetter.org
Date: 2025-01-30T12:02:19Z
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 Tue, Jan 28, 2025 at 9:02 AM Tatsuo Ishii <ishii@postgresql.org> wrote:
>
> >> +/*
> >> + * ignorenulls_getfuncarginframe
> >> + * For IGNORE NULLS, get the next nonnull value in the frame, moving forward or backward
> >> + * until we find a value or reach the frame's end.
> >> + */
> >> +static Datum
> >> +ignorenulls_getfuncarginframe(WindowObject winobj, int argno,
> >>
> >> Do you assume that win_nonnulls is sorted by pos? I think it's
> >> necessarily true that pos in win_nonnulls array is sorted. Is that ok?
> >
> > Yes it must be sorted on my understanding of the code.
>
> Then the patch has a problem. I ran a query below and examined
> win_nonnulls.  It seems it was not sorted out.
>
> SELECT
>   x,y,
>   nth_value(y,1) IGNORE NULLS OVER w
> FROM (VALUES (1,1), (2,2), (3,NULL), (4,4), (5,NULL), (6,6), (7,7)) AS t(x,y)
> WINDOW w AS (ORDER BY x ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING EXCLUDE CURRENT ROW);
>
>
> (gdb) p *winobj->win_nonnulls @ winobj->nonnulls_len
> $8 = {1, 0, 3, 6, 5}
>

I've looked at it again and I think the code is correct, but I
miswrote that the array needs to be sorted. The above query returns:
 x | y | nth_value
---+---+-----------
 1 | 1 |         2
 2 | 2 |         1
 3 |   |         2
 4 | 4 |
 5 |   |         4
 6 | 6 |         7
 7 | 7 |         6
(7 rows)

This is correct, for values of x:

1: The first non-null value of y is at position 0, however we have
EXCLUDE CURRENT ROW so it picks the next non-null value at position 1
and stores it in the array, returning 2.
2: We can now take the first non-null value of y at position 0 and
store it in the array, returning 1.
3. We take 1 preceding, using the position stored in the array, returning 2.
4. 1 preceding and 1 following are both null, and we exclude the
current row, so returning null.
5. 1 preceding is at position 3, store it in the array, returning 4.
6. 1 preceding is null and we exclude the current row, so store
position 6 in the array, returning 7.
7. 1 preceding is at position 5, store it in the array and return 6.

It will be unordered when the EXCLUDE clause is used but the code
should handle this correctly.