Re: [PATCH] Generate random dates/times in a specified range - Mailing list pgsql-hackers

Patch review

OVERALL:

I like the idea, especially the date variant. Unlike Tom, I'm not particularly concerned about breakage of existing scripts, as
most already are working just fine with raw numbers and I don't see this patch breaking them.

In a selfish vein, I would use the "date" and timestamp variants a lot. I would use the "time" ones seldom to never.

HOUSEKEEPING:

* Needs a pgindent run

* Commit message typo: developped

DOCS:

* Suggest adding an example of a random(date, date) call at the end of the <row> section

* Typo: speficy

* Suggest: "current session's timezone"

* Suggest: remove "instead"

CODE:

* Code comments should be consistent with "Returns" not "Generate"

* result = (DateADT) (int32) pg_prng_int64_range(&prng_state, rmin, rmax);

Is the int32 cast needed here? My compiler seems fine without it

*  This behaviour is based on generate_series_timestamptz_internal

Is it? I'm not seeing much overlap between the two

The addition of a timezone arg does complicate things. Is it really needed?

If not (just relying on session), timetzrandom becomes much much simpler (e.g. time_timetz)

RUNNING:

* Found a bug:

greg=# select random('12:00'::time, '13:00'::time);
     random
-----------------
 12:41:35.612385

greg=# select random('12:00'::time, '13:00'::time, ''::text);
       random
--------------------
 12:20:58.308452-04

greg=# select random('12:00'::time, '13:00'::time, 'FOO'::text);
ERROR:  time zone "FOO" not recognized

greg=# select random('12:00'::time, '13:00'::time, 'IST-9'::text);
       random
--------------------
 12:42:46.384039+09

greg=# select random('12:00'::time, '13:00'::time, 'IST'::text);
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.

At the very least, add a simple TLA timezone to the sql tests once fixed.


* Infinity

Fun, but is it useful? Perhaps disallow infinity. No strong opinion on this.

greg=# select random(now(), 'infinity'::date);
             random
--------------------------------
 86609-03-21 15:23:43.291271-04

greg=# select random('-infinity'::date, 'infinity'::date);
    random
---------------
 6126327-09-13