Re: [PATCH] pg_sleep(interval)

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Vik Fearing <vik(dot)fearing(at)dalibo(dot)com>
Cc: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] pg_sleep(interval)
Date: 2013-09-20 11:59:09
Message-ID: alpine.DEB.2.02.1309201351050.15741@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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 the non
regression tests.

- some concerns have been raised that it breaks pg_sleep(TEXT)
which currently works thanks to the implicit TEXT -> INT cast.

I would suggest to add pg_sleep(TEXT) explicitely, like:

$$ 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.


In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-09-20 11:59:30 Re: Assertions in PL/PgSQL
Previous Message Andres Freund 2013-09-20 11:57:28 Re: SSI freezing bug