On Fri, Sep 20, 2013 at 7:59 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> - 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:
>
> 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.
I think that's ugly and I'm not one bit convinced it will resolve all
the upgrade-compatibility issues. Realistically, all sleeps are going
to be reasonably well measured in seconds anyway. If you want to
sleep for some other interval, convert that interval to a number of
seconds first.
Another problem is that, as written, this is vulnerable to search_path
hijacking attacks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company