> On Oct 16, 2025, at 18:17, Tatsuo Ishii <ishii@postgresql.org> 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.
Afterthat, the function is immediately called again, so “datum” is overwritten. But I haven’t spent time thinking about
howto fix.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
