> On Oct 16, 2025, at 18:17, Tatsuo Ishii <[email protected]> wrote:
>
> Thanks for the report.
>
>> Coverity thinks that this code has still some incorrect bits, and I
>> think that it is right to think so even on today's HEAD at
>> 02c171f63fca.
>>
>> In WinGetFuncArgInPartition()@nodeWindowAgg.c, we have the following
>> loop (keeping only the relevant parts:
>> do
>> {
>> [...]
>> else /* need to check NULL or not */
>> {
>> /* get tuple and evaluate in partition */
>> datum = gettuple_eval_partition(winobj, argno,
>> abs_pos, isnull, &myisout);
>> if (myisout) /* out of partition? */
>> break;
>> if (!*isnull)
>> notnull_offset++;
>> /* record the row status */
>> put_notnull_info(winobj, abs_pos, *isnull);
>> }
>> } while (notnull_offset < notnull_relpos);
>>
>> /* get tuple and evaluate in partition */
>> datum = gettuple_eval_partition(winobj, argno,
>> abs_pos, isnull, &myisout);
>>
>> And Coverity is telling that there is no point in setting a datum in
>> this else condition to just override its value when we exit the while
>> loop. To me, it's a sigh that this code's logic could be simplified.
>
> To fix the issue, I think we can change:
>
>> datum = gettuple_eval_partition(winobj, argno,
>> abs_pos, isnull, &myisout);
>
> to:
>
> (void) gettuple_eval_partition(winobj, argno,
> abs_pos, isnull, &myisout);
>
> This explicitely stats that we ignore the return value from
> gettuple_eval_partition. I hope coverity understands this.
>
>>
I think Coverity is complaining about the redundant call to
gettuple_eval_partition().
In the “else” clause, the function is called, then when “if (myisout)” is
satisfied, it will break out the while loop. After that, the function is
immediately called again, so “datum” is overwritten. But I haven’t spent time
thinking about how to fix.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
