Re: Questionable coding in nth_value

> On Sat, May 6, 2023 at 4:44 PM Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)jp> wrote:
>
>> Currently Window function nth_value is coded as following:
>>
>> nth = DatumGetInt32(WinGetFuncArgCurrent(winobj, 1, &isnull));
>> if (isnull)
>> PG_RETURN_NULL();
>> const_offset = get_fn_expr_arg_stable(fcinfo->flinfo, 1);
>>
>> if (nth <= 0)
>> ereport(ERROR,
>> :
>> :
>>
>> Is there any reason why argument 'nth' is not checked earlier?
>> IMO, it is more natural "if (nth <= 0)..." is placed right after "nth =
>> DatumGetInt32...".
>>
>> Attached is the patch which does this.
>
>
> Hmm, shouldn't we check if the argument of nth_value is null before we
> check if it is greater than zero? So maybe we need to do this.

That makes sense. I thought since this function is marked as strict,
it would not be called if argument is NULL, but I was wrong.

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