> On Sat, May 6, 2023 at 4:44 PM Tatsuo Ishii <is...@sraoss.co.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