Re: New function normal_rand_array function to contrib/tablefunc.

On Tue, 27 Aug 2024 at 16:43, Andy Fan <zhihuifan1213(at)163(dot)com> wrote:
> Andy Fan <zhihuifan1213(at)163(dot)com> writes:
>
>>>> My suggestion would be to mirror the signatures of the core random()
>>>> functions more closely, and have this:
>>>>
>>>> 1). rand_array(numvals int, minlen int, maxlen int)
>>>> returns setof float8[]
>>>>
>> ..>
>>>> 4). rand_array(numvals int, minlen int, maxlen int,
>>>> minval numeric, maxval numeric)
>>>> returns setof numeric[]
>>
>>> this is indeed a more clean and correct APIs, I will use the above ones
>>> in the next version. Thanks for the suggestion.
>>
>> I followed your suggestion in the new attached version. They are not
>> only some cleaner APIs for user and but also some cleaner implementation
>> in core, Thank for this suggestion as well.
>
> A new version is attached, nothing changed except replace
> PG_GETARG_INT16 with PG_GETARG_INT32. PG_GETARG_INT16 is a copy-paste
> error.
>

Thanks for updating the patch. Here are some comments.

+ if (minlen >= maxlen)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("minlen must be greater than maxlen.")));

There error message should be "minlen must be smaller than maxlen", right?

+ if (minlen < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("minlen and maxlen must be greater than zero.")));

Here the minlen might be zero, so the error message is incorrect.
How about use "minlen must be greater than or equal to zero"?

--
Regrads,
Japin Li