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.
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index b87a624fb2..f4ff060930 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -696,15 +696,16 @@ window_nth_value(PG_FUNCTION_ARGS)
int32 nth;
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,
(errcode(ERRCODE_INVALID_ARGUMENT_FOR_NTH_VALUE),
errmsg("argument of nth_value must be greater than zero")));
+ if (isnull)
+ PG_RETURN_NULL();
+
+ const_offset = get_fn_expr_arg_stable(fcinfo->flinfo, 1);
+
result = WinGetFuncArgInFrame(winobj, 0,
nth - 1, WINDOW_SEEK_HEAD, const_offset,
&isnull, NULL);