[PATCH] pg_sleep(interval)

Usage: SELECT pg_sleep(interval '2 minutes');

I would add this to the next commitfest but I seem to be unable to log
in with my community account (I can log in to the wiki). Help appreciated.

Done.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Aug 8, 2013 at 1:52 PM, Vik Fearing <vik.fearing@dalibo.com> wrote:

Someone on IRC a while ago was complaining that there was no way to
specify an interval for pg_sleep, so I made one. Patch against today's
HEAD attached.

Usage: SELECT pg_sleep(interval '2 minutes');

I would add this to the next commitfest but I seem to be unable to log
in with my community account (I can log in to the wiki). Help appreciated.

Try changing your password on the main website. There was a bug a
while ago (a few months iirc) and if you changed your password or
created your account during that period, it would not work for the cf
app (but it would work for the wiki and some other sites).

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Aug 8, 2013 at 7:52 AM, Vik Fearing <vik.fearing@dalibo.com> wrote:

Someone on IRC a while ago was complaining that there was no way to
specify an interval for pg_sleep, so I made one. Patch against today's
HEAD attached.

Usage: SELECT pg_sleep(interval '2 minutes');

The problem with this is that then pg_sleep would be overloaded, and
as we've found from previous experience (cf. pg_size_pretty) that can
make things that currently work start failing as ambiguous.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Except there are no data types that can be cast to both double and
interval currently.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark <stark@mit.edu> wrote:

Except there are no data types that can be cast to both double and
interval currently.

That, unfortunately, is not sufficient to avoid a problem.

rhaas=# create or replace function foo(double precision) returns
double precision as $$select $1$$ language sql;
CREATE FUNCTION
rhaas=# create or replace function foo(interval) returns interval as
$$select $1$$ language sql;
CREATE FUNCTION
rhaas=# select foo('123');
ERROR: function foo(unknown) is not unique
LINE 1: select foo('123');
^
HINT: Could not choose a best candidate function. You might need to
add explicit type casts.
rhaas=#

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/16/13 3:35 PM, Robert Haas wrote:

On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark <stark@mit.edu> wrote:

Except there are no data types that can be cast to both double and
interval currently.

That, unfortunately, is not sufficient to avoid a problem.

rhaas=# create or replace function foo(double precision) returns
double precision as $$select $1$$ language sql;
CREATE FUNCTION
rhaas=# create or replace function foo(interval) returns interval as
$$select $1$$ language sql;
CREATE FUNCTION
rhaas=# select foo('123');
ERROR: function foo(unknown) is not unique
LINE 1: select foo('123');
^
HINT: Could not choose a best candidate function. You might need to
add explicit type casts.

That example can be used as an argument against almost any kind of
overloading.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

On 8/16/13 3:35 PM, Robert Haas wrote:

On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark <stark@mit.edu> wrote:

Except there are no data types that can be cast to both double and
interval currently.

That, unfortunately, is not sufficient to avoid a problem.

That example can be used as an argument against almost any kind of
overloading.

True. I think the gripe here is that pg_sleep('42') has worked for
many releases now, and if we add this patch then it would suddenly
stop working. How common is that usage likely to be (probably not
very), and how useful is it to have a version of pg_sleep that
takes an interval (probably also not very)?

Since the same effect can be had by writing a user-defined SQL function,
I'm a bit inclined to say that the value-added by having this as a
built-in function doesn't justify the risk of breaking existing apps.
It's a close call though, because both the risk and the value-added seem
rather small from here.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/16/2013 04:52 PM, Tom Lane wrote:

Since the same effect can be had by writing a user-defined SQL function,
I'm a bit inclined to say that the value-added by having this as a
built-in function doesn't justify the risk of breaking existing apps.
It's a close call though, because both the risk and the value-added seem
rather small from here.

Why not just call it pg_sleep_int()?

I, for one, would find it useful, but would be really unhappy about
about having to debug a bunch of old code to figure out what was broken.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Josh Berkus <josh@agliodbs.com> writes:

Why not just call it pg_sleep_int()?

To me, that looks like something that would take an int. I suppose you
could call it pg_sleep_interval(), but that's getting pretty verbose.

The larger picture here though is that that's ugly as sin; it just flies
in the face of the fact that PG *does* have function overloading and we
do normally use it, not invent randomly-different function names to avoid
using it. We should either decide that this feature is worth the small
risk of breakage, or reject it. Not build a camel-designed-by-committee
because no one would speak up for consistency of design.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 08/16/2013 05:15 PM, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Why not just call it pg_sleep_int()?

To me, that looks like something that would take an int. I suppose you
could call it pg_sleep_interval(), but that's getting pretty verbose.

The larger picture here though is that that's ugly as sin; it just flies
in the face of the fact that PG *does* have function overloading and we
do normally use it, not invent randomly-different function names to avoid
using it. We should either decide that this feature is worth the small
risk of breakage, or reject it. Not build a camel-designed-by-committee
because no one would speak up for consistency of design.

Well, if that's the alternative, I'd vote for taking it. For me,
personally, I think the usefulness of it would outstrip the number of
functions I'd have to debug.

For one thing, it's not like pg_sleep is exactly widely used, especially
not from languages like PHP which tend to treat every variable as a
string. So this is not going to be the kind of upgrade bomb that
pg_size_pretty was.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Aug 16, 2013 at 4:16 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

That example can be used as an argument against almost any kind of
overloading.

Yep.

And that may be justified. We don't handle overloading particularly well.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

True. I think the gripe here is that pg_sleep('42') has worked for
many releases now,

Maybe it could still work if pg_sleep(TEXT) is added as well?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Josh Berkus (josh@agliodbs.com) wrote:

Well, if that's the alternative, I'd vote for taking it. For me,
personally, I think the usefulness of it would outstrip the number of
functions I'd have to debug.

Agreed. Having it take an interval makes a whole lot more sense than
backing a decision that it should take a string-cast-to-integer.

Thanks,

Stephen

On 8/16/13 7:52 PM, Tom Lane wrote:

I think the gripe here is that pg_sleep('42') has worked for
many releases now, and if we add this patch then it would suddenly
stop working. How common is that usage likely to be (probably not
very), and how useful is it to have a version of pg_sleep that
takes an interval (probably also not very)?

I think it's always going to be a problem going from a function with
only one signature to more than one. It's not going to be a problem
going from two to more.

For example, if you had foo(point) and much later you want to add
foo(box), someone might complain that foo('(1,2)') has worked for many
releases now, and how common is that use? If we had started out with
foo(point) and foo(line) simultaneously, this wouldn't have become a
problem.

This is quite a silly situation. I don't know a good answer, except
either ignoring the problem or requiring that any new function has at
least two overloaded variants. ;-)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

For example, if you had foo(point) and much later you want to add
foo(box), someone might complain that foo('(1,2)') has worked for many
releases now, and how common is that use? If we had started out with
foo(point) and foo(line) simultaneously, this wouldn't have become a
problem.

You may provide foo(TEXT) along foo(box) so that foo('(1,2)') works as it
did before.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/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:

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello Robert,

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

I do not know that. From a "usual" dabatabase point of view, it does not
make much sense to slow down a database anyway, and this function is never
needed... so it really depends on the use case.

If someone want to simulate a long standing transaction to check its
effect on some features such as dumping data orreplication or whatever,
maybe pg_sleep(INTERVAL '5 hours') is nicer that pg_sleep(18000), if you
are not too good at dividing by 60, 3600 or 86400...

If you want to sleep for some other interval, convert that interval to a
number of seconds first.

You could say that for any use of INTERVAL. ISTM that the point if the
interval type is just to be more readable than a number of seconds to
express a delay.

Another problem is that, as written, this is vulnerable to search_path
hijacking attacks.

Yes, sure. I was not suggesting to create the function directly as above,
this is just the test I made to check whether it worked as I thought, i.e.
providing a TEXT version works and interacts properly with the INTEGER and
INTERVAL versions. My guess is that the function definition would be
inserted directly in pg_proc as other pg_* functions at initdb time.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/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