Re: [PATCH] Introduce array_shuffle() and array_sample()

>> i came to the same conclusions and went with Option 1 (see patch). Mainly
>> because most code in utils/adt is organized by type and this way it is
>> clear, that this is a thin wrapper around pg_prng.
>>
>
> Small patch update. I realized the new functions should live
> array_userfuncs.c (rather than arrayfuncs.c), fixed some file headers and
> added some comments to the code.

My 0,02€ about this patch:

Use (void) when declaring no parameters in headers or in functions.

Should the exchange be skipped when i == k?

I do not see the point of having *only* inline functions in a c file. The
external functions should not be inlined?

The random and array shuffling functions share a common state. I'm
wondering whether it should ot should not be so. It seems ok, but then
ISTM that the documentation suggests implicitely that setseed applies to
random() only, which is not the case anymore after the patch.

If more samples are required than the number of elements, it does not
error out. I'm wondering whether it should.

Also, the sampling should not return its result in order when the number
of elements required is the full array, ISTM that it should be shuffled
there as well.

I must say that without significant knowledge of the array internal
implementation, the swap code looks pretty strange. ISTM that the code
would be clearer if pointers and array references style were not
intermixed.

Maybe you could add a test with a 3D array? Some sample with NULLs?

Unrelated: I notice again that (postgre)SQL does not provide a way to
generate random integers. I do not see why not. Should we provide one?

--
Fabien.