Here is a review of the pg_sleep(INTERVAL) patch version 1:
- patch applies cleanly on current head
- the function documentation is updated and clear
- the function definition relies on a SQL function to call the underlying pg_sleep(INT) function I'm fine with
this,especially as if someone calls this function, he/she is not in a hurry:-)
- this one-liner implementation is straightforward
- the provided feature is simple, and seems useful.
- configure/make/make check work ok
However :
- the new function is *not* tested anywhere!
I would suggest simply to replace some pg_sleep(int) instances by corresponding pg_sleep(interval) instances in
thenon regression tests.
- some concerns have been raised that it breaks pg_sleep(TEXT) which currently works thanks to the implicit TEXT ->
INTcast.
I would suggest to add pg_sleep(TEXT) explicitely, like:
CREATE FUNCTION pg_sleep(TEXT) RETURNS VOID VOLATILE STRICT AS $$ select pg_sleep($1::INTEGER) $$ LANGUAGE
SQL;
That would be another one liner, to update the documentation and to add some tests as well!
ISTM that providing "pg_sleep(TEXT)" cleanly resolves the upward-compatibility issue raised.
--
Fabien
