From: | Vik Fearing <vik(dot)fearing(at)dalibo(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | PG Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] pg_sleep(interval) |
Date: | 2013-09-22 11:13:35 |
Message-ID: | 523ED0DF.3010007@dalibo.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 09/20/2013 01:59 PM, Fabien COELHO wrote:
>
> Here is a review of the pg_sleep(INTERVAL) patch version 1:
Thank you for looking at it.
>
> - 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.
Attached is a rebased patch that adds a test as you suggest.
> - some concerns have been raised that it breaks pg_sleep(TEXT)
> which currently works thanks to the implicit TEXT -> INT cast.
There is no pg_sleep(text) function and the cast is unknown->double
precision.
>
> 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 don't like this idea at all. If we don't want to break compatibility
for callers that quote the value, I would rather the patch be rejected
outright.
--
Vik
Attachment | Content-Type | Size |
---|---|---|
pg_sleep_interval.v2.patch | text/x-patch | 3.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2013-09-22 12:17:37 | Re: [PATCH] pg_sleep(interval) |
Previous Message | Fabien COELHO | 2013-09-22 10:07:09 | Re: pgbench progress report improvements |