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