From: | Paul Ramsey <pramsey(at)cleverelephant(dot)ca> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] random_normal function |
Date: | 2022-12-09 17:17:03 |
Message-ID: | 1D4DA54E-76BE-4A38-9313-BEC9FE231543@cleverelephant.ca |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Dec 8, 2022, at 8:29 PM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Dec 08, 2022 at 04:44:56PM -0800, Paul Ramsey wrote:
>> Final tme, with fixes from cirrusci.
>
> Well, why not. Seems like you would use that a lot with PostGIS.
>
> #include <math.h> /* for ldexp() */
> +#include <float.h> /* for DBL_EPSILON */
> And be careful with the order here.
Should be ... alphabetical?
> +static void
> +drandom_check_default_seed()
> We always use (void) rather than empty parenthesis sets.
OK
> I would not leave that unchecked, so I think that you should add
> something in ramdom.sql. Or would you prefer switching some of
> the regression tests be switched so as they use the new normal
> function?
Reading through those tests... seems like they will (rarely) fail. Is that... OK?
The tests seem to be mostly worried that random() starts returning constants, which seems like a good thing to test for (is the random number generating returning randomness).
An obvious test for this function is that the mean and stddev converge on the supplied parameters, given enough inputs, which is actually kind of the opposite test. I use the same random number generator as the uniform distribution, so that aspect is already covered by the existing tests.
> (Ahem. Bonus points for a random_string() returning a bytea, based on
> pg_strong_random().)
Would love to. Separate patch of bundled into this one?
P
> --
> Michael
From | Date | Subject | |
---|---|---|---|
Next Message | samay sharma | 2022-12-09 18:10:21 | Re: PGDOCS - Logical replication GUCs - added some xrefs |
Previous Message | Andres Freund | 2022-12-09 16:58:20 | Re: Meson add host_system to PG_VERSION_STR |