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

Tatsuo Ishii <ishii@postgresql.org>

From: Tatsuo Ishii <ishii@postgresql.org>
To: tgl@sss.pgh.pa.us
Cc: ojford@gmail.com, peter@eisentraut.org, li.evan.chao@gmail.com, krasiyan@gmail.com, vik@postgresfriends.org, andrew@tao11.riddles.org.uk, david@fetter.org, pgsql-hackers@lists.postgresql.org
Date: 2025-10-05T23:51:33Z
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

Attachments

Thank you for the report!

> Coverity is not very happy with this patch.
> It's complaining that the result of window_gettupleslot
> is not checked, which seems valid:
> 
> 1503     			{
> 1504     				if (fetch_tuple)
>>>>     CID 1666587:         Error handling issues  (CHECKED_RETURN)
>>>>     Calling "window_gettupleslot" without checking return value (as is done elsewhere 8 out of 9 times).
> 1505     					window_gettupleslot(winobj, pos, slot);
> 1506     				if (!are_peers(winstate, slot, winstate->ss.ss_ScanTupleSlot))
> 1507     					return -1;

Yes, I forgot to check the return value of window_gettupleslot.

> and also that WinGetFuncArgInPartition is dereferencing
> a possibly-null "isout" pointer at several places, including
> 
>>>>     Dereferencing null pointer "isout".
> 3806     				if (*isout)		/* out of partition? */
> 
>>>>     Dereferencing null pointer "isout".
> 3817     	if (!*isout && set_mark)
> 3818     		WinSetMarkPosition(winobj, abs_pos);
> 
>>>>     Dereferencing null pointer "isout".
> 3817     	if (!*isout && set_mark)
> 3818     		WinSetMarkPosition(winobj, abs_pos);
> 
> The latter complaints seem to be because some places in
> WinGetFuncArgInPartition check for nullness of that pointer
> and some do not.  That looks like at least a latent bug
> to me.

Agreed.

Attached is a patch to fix the issue.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp