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

From: Tatsuo Ishii <ishii(at)postgresql(dot)org>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: ojford(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com, krasiyan(at)gmail(dot)com, vik(at)postgresfriends(dot)org, andrew(at)tao11(dot)riddles(dot)org(dot)uk, david(at)fetter(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date: 2025-10-07 02:28:32
Message-ID: 20251007.112832.740065769089328041.ishii@postgresql.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>> 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.
>
> Please disregard the v1 patch. It includes a bug: If
> WinGetFuncArgInPartition() is called with set_mark == true and isout
> == NULL, WinSetMarkPosition() is not called by
> WinGetFuncArgInPartition().
>
> I will post v2 patch.

Attached is the v2 patch.

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

Attachment Content-Type Size
v2-0001-Fix-Coverity-issues-reported-in-commit-25a30bbd42.patch application/octet-stream 4.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2025-10-07 03:12:06 Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Previous Message Hayato Kuroda (Fujitsu) 2025-10-07 02:00:18 RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE