Re: [HACKERS] [PATCH] pg_sleep(interval)
On Sun, Feb 2, 2014 at 4:50 AM, Julien Rouhaud rjuju...@gmail.com wrote: It seems like pg_sleep_until() has issues if used within a transaction, as it uses now() and not clock_timestamp(). Please find attached a patch that solves this issue. For consistency reasons, I also modified pg_sleep_for() to also use clock_timestamp. Good catch! Committed. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hi
It seems like pg_sleep_until() has issues if used within a transaction, as
it uses now() and not clock_timestamp(). Please find attached a patch that
solves this issue.
For consistency reasons, I also modified pg_sleep_for() to also use
clock_timestamp.
Regards
On Fri, Jan 31, 2014 at 2:12 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 01/30/2014 09:48 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing vik.fear...@dalibo.com
wrote:
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com
wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
Committed.
Thanks!
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 3034,3042 DATA(insert OID = 2625 ( pg_ls_dir PGNSP
PGUID 12 1 1000 0 0 f f f f t t v 1 0
DESCR(list all files in a directory);
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0
f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_
_null_ ));
DESCR(sleep for the specified time in seconds);
! DATA(insert OID = 3935 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0
f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select
pg_catalog.pg_sleep(extract(epoch from pg_catalog.now() operator(pg_catalog.+)
$1) operator(pg_catalog.-) extract(epoch from pg_catalog.now())) _null_ _null_
_null_ ));
DESCR(sleep for the specified interval);
! DATA(insert OID = 3936 ( pg_sleep_until PGNSP PGUID 14
1 0 0 0 f f f f t f v 1 0 2278 1184 _null_ _null_ _null_ _null_ select
pg_catalog.pg_sleep(extract(epoch from $1) operator(pg_catalog.-) extract(epoch
from pg_catalog.now())) _null_ _null_ _null_ ));
DESCR(sleep until the specified time);
DATA(insert OID = 2971 ( textPGNSP PGUID 12
1 0 0 0 f f f f t f i 1 0 25 16 _null_ _null_ _null_ _null_ booltext _null_
_null_ _null_ ));
--- 3034,3042
DESCR(list all files in a directory);
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0
f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_
_null_ ));
DESCR(sleep for the specified time in seconds);
! DATA(insert OID = 3935 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0
f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select
pg_catalog.pg_sleep(extract(epoch from pg_catalog.clock_timestamp()
operator(pg_catalog.+) $1) operator(pg_catalog.-) extract(epoch from
pg_catalog.clock_timestamp())) _null_ _null_ _null_ ));
DESCR(sleep for the specified interval);
! DATA(insert OID = 3936 ( pg_sleep_until PGNSP PGUID 14
1 0 0 0 f f f f t f v 1 0 2278 1184 _null_ _null_ _null_ _null_ select
pg_catalog.pg_sleep(extract(epoch from $1) operator(pg_catalog.-) extract(epoch
from pg_catalog.clock_timestamp())) _null_ _null_ _null_ ));
DESCR(sleep until the specified time);
DATA(insert OID = 2971 ( textPGNSP PGUID 12
1 0 0 0 f f f f t f i 1 0 25 16 _null_ _null_ _null_ _null_ booltext _null_
_null_ _null_ ));
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
Committed.
--
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 01/30/2014 09:48 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
Committed.
Thanks!
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Fri, Oct 18, 2013 at 7:21 PM, Jim Nasby j...@nasby.net wrote: Yeah, but hasn't every case of this that we've run into been directly related to casting problems, and not function or operator preference? No. Something else I'm wondering is if priority should actually be something that's numbered instead of just a boolean. I can see far more logic to implicitly casting text to double than I can text to interval, but if a cast to double won't actually get you where you want and a cast to interval will... Maybe it's possible to account for all those cases with just a boolean... maybe not. I wondered about this, too. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Vik, * Vik Fearing (vik.fear...@dalibo.com) wrote: A parser refactoring broke my code. I reported it and it was promptly fixed. When the fix came up for review, I said it needed a regression test to prevent it from happening again and I was told by the author that such a test would be flimsy and it went on to be committed (by that same guy) without one. I'm undecided whether I'll be contributing there any further. To be fair, we've declined to add regression tests, and in cases even removed them, if they have been a source of false positives or add a lot of extra time to the regression suite without really adding a lot of coverage. We continue to discuss, and need imv, more regression tests, grouped into different sets which are run depending on the environment and local configuration. Running the regression suite while doing quick code iterations should be fast, while the build farm should run as many regression tests as practical (which might depend on the system). There has been some progress on this front recently by Peter E and Andrew Dunstan, as I recall, but I've not followed it very closely. They may very well already have this set up. The rigor here makes me want to try and try again. This is certainly good to hear and I hope that you continue to contribute and keep on trying. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 01:41 PM, Vik Fearing wrote: Perhaps; but it has also been an example of the benefits of having tight review. FWIW, I agree. I have been impressed by the rigorous review process of this project ever since I started following it. OK, good! That makes me feel better. So, I surveyed 30 members of the San Francisco PostgreSQL User Group last night. Out of the 30: 4 had ever used pg_sleep(), and those four included Jeff Davis and Peter G. I asked the remaining two about the new versions of pg_sleep, and they were more interested in pg_sleep_until(), and not particularly interested in pg_sleep(interval). So, to my mind backwards compatibility (the ambiguity issue) is insignificant because there are so few users of pg_sleep(), but there are serious questions about the demand for improvements on pg_sleep for that reason. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/13 12:10 PM, Josh Berkus wrote: On 10/17/2013 10:01 AM, Robert Haas wrote: But if you're asking my opinion, I think doing it on the function level is a whole lot better and easier to get right. A flag like the one I mentioned here can be set for one particular function with the absolute certainty that behavior will not change for any function with some other name. That type of surety is pretty much impossible to get with casts. The other argument for doing it at the function level is that we could then expose it to users, who could use it to manage their own overloaded functions. We would NOT want to encourage users to mess with cast precedence, because it would be impossible for them to achieve their desired result that way. On the other hand, prioritization at the function level likely wouldn't help us with operators at all, because there the cast has to be chosen before we choose a function. So if we pursued the function route, then we'd eventually want to add a preferred flag for operators too. Which would be a lot more trouble, because it would affect the planner, but at least that would be a seperate step. Yeah, but hasn't every case of this that we've run into been directly related to casting problems, and not function or operator preference? ISTM that exposing the idea of function priority to users is opening a massive Pandora's box... Something else I'm wondering is if priority should actually be something that's numbered instead of just a boolean. I can see far more logic to implicitly casting text to double than I can text to interval, but if a cast to double won't actually get you where you want and a cast to interval will... Maybe it's possible to account for all those cases with just a boolean... maybe not. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/13 4:01 PM, Vik Fearing wrote: On 10/17/2013 06:59 PM, Josh Berkus wrote: Our project has a serious, chronic problem with giving new patch-submitters a bad experience, and this patch is a good example of that. The ultimate result is that people go off to contribute to other projects where submissions are easier and the rules for what gets accepted are relatively transparent. That may be true, but it depends on the contributor. I would much rather be told that my contribution is not up to snuff than what happened on another project I recently tried to contribute to for the first time. A parser refactoring broke my code. I reported it and it was promptly fixed. When the fix came up for review, I said it needed a regression test to prevent it from happening again and I was told by the author that such a test would be flimsy and it went on to be committed (by that same guy) without one. I'm undecided whether I'll be contributing there any further. The rigor here makes me want to try and try again. ISTM the big issue with new contributors is our methodology is rather different from most other projects, and if you don't understand that you're likely to end up with negativity towards contributing here. Specifically: - We place a heavy, HEAVY emphasis on discussion, to the point that you can easily spend 50x more time on discussing a feature over implementing it. - We place a very heavy emphasis on quality, be that testing, not breaking backwards compatability, etc, etc. I agree with Vik; I think the way we develop is a feature and not a bug. But I also think we need to do everything we can to enlighten new contributors so they don't walk away with a bad taste in their mouth. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hello Vik, For: Josh, Stephen, me Against: Robert Neutral: Tom, you For the record, I'm not neutral, I'm *FOR*. I reviewed it and said that I think it is fine. But I'm still nobody here:-) My experience at trying to pass minor patches shows that the basic behavior is conservatism. Maybe this is necessary to the stability of the project, but this is really annoying for pretty small changes on side tools, and I think that it tends to over-conservatism and ampers good will. You have to argue a lot about trivial things. My ratio for passing very minor patches on pgbench for instance is 1:3 or worst, 1 unit programming and testing versus 3 units writing mails to argue about this and that. Maybe the ratio is better for big important patches. Your patch is quite representative, 1 line of trivial code, a few lines of tests and docs, and how many lines and time at writing mails? I don't know if that's enough of a consensus to commit it, but I do think it's not nearly enough of a consensus to reject it. My guess is that it won't be committed if there is a single but it might break one code or surprise one user somewhere in the universe, but I wish I'll be proven wrong. IMO, returned with feedback on a 1 liner is really akin to rejected. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Wed, Oct 16, 2013 at 5:16 PM, Peter Eisentraut pete...@gmx.net wrote:
On 10/16/13 2:40 PM, Robert Haas wrote:
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus j...@agliodbs.com wrote:
Also, as Tom pointed out, at some point we have to either say we really
support overloading or we don't.
We clearly do support overloading. I don't think that's at issue.
But as we all know, using it can cause formerly unambiguous queries to
become ambiguous and stop working.
But that's not really what this is. It's one thing to be wary about
adding foo(bigint, int, smallint) when there are already three other
permutations in the system. (At least in other languages, compilers
will give you warnings or errors when this creates an ambiguity, so
there's no guessing.) But the problem here is that if there already is a
foo(type1)
then the proposal to add
foo(type2)
can always be shot down by
But this will break foo('type1val').
That can't be in the spirit of overloading.
The only way to fix this is that at the time when you add foo(type1) you
need to prevent people from being able to call foo('type1val') and
instead require the full syntax foo(type1 'type1val'). The only way to
do that, I think, is to add some other foo(type3) at the same time.
There is just something wrong with that.
Actually, this could be fixed by having a way to declare one of the
overloaded functions as the preferred option and resolving ambiguous
calls in favor of the highest-priority function. In fact,
EnterpriseDB has added just such an option to Advanced Server 9.3, and
it fixes several longstanding difficult choices between being
Oracle-compatible and being PostgreSQL-compatible; we're now more
compatible with both. If we had that option in PostgreSQL, we could
declare the existing function as the preferred option, add the new
one, and be pretty much assured of not breaking anything.
However, I've learned from experience that submitting patches to
improve PostgreSQL's only-marginally-usable ambiguous function
resolution code is an exercise in futility. My last attempt ended
with a clear majority of people telling me that they liked failing to
call *the only function called foo* when confronted with a call that
was clearly intended to reference that function. As nearly as I can
tell, people like the idea of such unnecessary failures only in
theory. As soon as it comes to any practical case (like this one),
people start looking for workarounds. Right now there aren't any good
ones, and the reason for that is simple: we refuse to entertain adding
them.
Just sayin'.
--
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
Opinions?
--
Vik
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 7586,7605 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT
/indexterm
para
! The following function is available to delay execution of the server
process:
synopsis
pg_sleep(replaceableseconds/replaceable)
/synopsis
functionpg_sleep/function makes the current session's process
sleep until replaceableseconds/replaceable seconds have
elapsed. replaceableseconds/replaceable is a value of type
typedouble precision/, so fractional-second delays can be specified.
For example:
programlisting
SELECT pg_sleep(1.5);
/programlisting
/para
--- 7586,7613
/indexterm
para
! The following functions are available to delay execution of the server
process:
synopsis
pg_sleep(replaceableseconds/replaceable)
+ pg_sleep_for(typeinterval/)
+ pg_sleep_until(typetimestamp with time zone/)
/synopsis
functionpg_sleep/function makes the current session's process
sleep until replaceableseconds/replaceable seconds have
elapsed. replaceableseconds/replaceable is a value of type
typedouble precision/, so fractional-second delays can be specified.
+ functionpg_sleep_for/function is a convenience function for larger
+ sleep times specified as an typeinterval/.
+ functionpg_sleep_until/function is a convenience function for when
+ a specific wake-up time is desired.
For example:
programlisting
SELECT pg_sleep(1.5);
+ SELECT pg_sleep_for('5 minutes');
+ SELECT pg_sleep_until('tomorrow 03:00');
/programlisting
/para
***
*** 7608,7622 SELECT pg_sleep(1.5);
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
/para
/note
warning
para
Make sure that your session does not hold more locks than necessary
! when calling functionpg_sleep/function. Otherwise other sessions
! might have to wait for your sleeping process, slowing down the entire
! system.
/para
/warning
/sect2
--- 7616,7632
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
+ In particular, functionpg_sleep_until/function is not guaranteed to
+ wake up exactly at the specified time, but it will not wake up any earlier.
/para
/note
warning
para
Make sure that your session does not hold more locks than necessary
! when calling functionpg_sleep/function or its variants. Otherwise
! other sessions might have to wait for your sleeping process, slowing down
! the entire system.
/para
/warning
/sect2
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0
--- 3017,3026
DESCR(list all files in a directory);
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ ));
DESCR(sleep for the specified time in seconds);
+ DATA(insert OID = 3179 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())) _null_ _null_ _null_ ));
+ DESCR(sleep for the specified interval);
+ DATA(insert OID = 3180 ( pg_sleep_until PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 1184 _null_ _null_ _null_ _null_ select pg_sleep(extract(epoch from $1) - extract(epoch from now())) _null_ _null_ _null_ ));
+ DESCR(sleep until the specified time);
DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 25 16 _null_ _null_ _null_ _null_
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
--
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
--
Vik
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 7586,7605 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT
/indexterm
para
! The following function is available to delay execution of the server
process:
synopsis
pg_sleep(replaceableseconds/replaceable)
/synopsis
functionpg_sleep/function makes the current session's process
sleep until replaceableseconds/replaceable seconds have
elapsed. replaceableseconds/replaceable is a value of type
typedouble precision/, so fractional-second delays can be specified.
For example:
programlisting
SELECT pg_sleep(1.5);
/programlisting
/para
--- 7586,7613
/indexterm
para
! The following functions are available to delay execution of the server
process:
synopsis
pg_sleep(replaceableseconds/replaceable)
+ pg_sleep_for(typeinterval/)
+ pg_sleep_until(typetimestamp with time zone/)
/synopsis
functionpg_sleep/function makes the current session's process
sleep until replaceableseconds/replaceable seconds have
elapsed. replaceableseconds/replaceable is a value of type
typedouble precision/, so fractional-second delays can be specified.
+ functionpg_sleep_for/function is a convenience function for larger
+ sleep times specified as an typeinterval/.
+ functionpg_sleep_until/function is a convenience function for when
+ a specific wake-up time is desired.
For example:
programlisting
SELECT pg_sleep(1.5);
+ SELECT pg_sleep_for('5 minutes');
+ SELECT pg_sleep_until('tomorrow 03:00');
/programlisting
/para
***
*** 7608,7622 SELECT pg_sleep(1.5);
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
/para
/note
warning
para
Make sure that your session does not hold more locks than necessary
! when calling functionpg_sleep/function. Otherwise other sessions
! might have to wait for your sleeping process, slowing down the entire
! system.
/para
/warning
/sect2
--- 7616,7632
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
+ In particular, functionpg_sleep_until/function is not guaranteed to
+ wake up exactly at the specified time, but it will not wake up any earlier.
/para
/note
warning
para
Make sure that your session does not hold more locks than necessary
! when calling functionpg_sleep/function or its variants. Otherwise
! other sessions might have to wait for your sleeping process, slowing down
! the entire system.
/para
/warning
/sect2
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0
--- 3017,3026
DESCR(list all files in a directory);
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ ));
DESCR(sleep for the specified time in seconds);
+ DATA(insert OID = 3179 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select pg_catalog.pg_sleep(extract(epoch from pg_catalog.now() operator(pg_catalog.+) $1) operator(pg_catalog.-) extract(epoch from pg_catalog.now())) _null_ _null_
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hello Vik,
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
Opinions?
I liked overloading...
Anyway, I have not looked at the patch in details, but both functions
seems useful at least if you need to sleep, and the solution is
reasonable.
I also like pg_sleep_until('tomorrow'); that I guess would work, but I'm
not sure of any practical use case for this one. Do you have an example
where it makes sense?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert Haas escribió: Actually, this could be fixed by having a way to declare one of the overloaded functions as the preferred option and resolving ambiguous calls in favor of the highest-priority function. In fact, EnterpriseDB has added just such an option to Advanced Server 9.3, and it fixes several longstanding difficult choices between being Oracle-compatible and being PostgreSQL-compatible; we're now more compatible with both. How does this work? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 9:51 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: Actually, this could be fixed by having a way to declare one of the overloaded functions as the preferred option and resolving ambiguous calls in favor of the highest-priority function. In fact, EnterpriseDB has added just such an option to Advanced Server 9.3, and it fixes several longstanding difficult choices between being Oracle-compatible and being PostgreSQL-compatible; we're now more compatible with both. How does this work? In Advanced Server, we've added implicit casts between some data types between which PostgreSQL does not have implicit casts. We do this because Oracle implicitly casts between those data types, and having similar casts allows function calls that would succeed on Oracle to also succeed on Advanced Server. Unfortunately, it also renders some operators, particularly textanycat and anytextcat, ambiguous. In existing releases of Advanced Server, we handled that by removing those operators. This creates some breakage in edge cases: concatenation with text still works fine for the data types for which we added implicit casts to text, but for other data types it fails where it would have succeeded in PostgreSQL. In Advanced Server 9.3, we added a new pg_proc column called proisweak. When a function or operator call would otherwise be rejected as ambiguous, we check whether all but one of the remaining candidates are marked proisweak; if so, we select the non-weak candidate. Advanced Server 9.3 now marks the textanycat and anytextcat operators as weak rather than removing them; this allows type-with-an-implicit-cast-to-text || text to work, because we now have a way to prefer implicit cast + textcat to anytextcat. Obviously, the implicit casts are not for PostgreSQL and would be rightly rejected here, but I am not sure that the ability to prefer one function or operator over others in an overloading situation is such a bad idea. So far, our internal testing seems to show that it works well and doesn't break things. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/13 8:06 AM, Robert Haas wrote: Actually, this could be fixed by having a way to declare one of the overloaded functions as the preferred option and resolving ambiguous calls in favor of the highest-priority function. In fact, EnterpriseDB has added just such an option to Advanced Server 9.3, and it fixes several longstanding difficult choices between being Oracle-compatible and being PostgreSQL-compatible; we're now more compatible with both. If we had that option in PostgreSQL, we could declare the existing function as the preferred option, add the new one, and be pretty much assured of not breaking anything. That might be worth discussing. I'd prefer somehow getting rid of the unknown literals/type, but that's a different discussion. However, I've learned from experience that submitting patches to improve PostgreSQL's only-marginally-usable ambiguous function resolution code is an exercise in futility. My last attempt ended with a clear majority of people telling me that they liked failing to call *the only function called foo* when confronted with a call that was clearly intended to reference that function. As nearly as I can tell, people like the idea of such unnecessary failures only in theory. As soon as it comes to any practical case (like this one), people start looking for workarounds. Right now there aren't any good ones, and the reason for that is simple: we refuse to entertain adding them. Well, that was a proposal to make things more loose, and it's reasonable to have issues with that. On the other hand, making things more strict is obviously prone to break existing code. So it's indeed difficult to make any changes either way. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert, Obviously, the implicit casts are not for PostgreSQL and would be rightly rejected here, but I am not sure that the ability to prefer one function or operator over others in an overloading situation is such a bad idea. So far, our internal testing seems to show that it works well and doesn't break things. Hmmm. Is this better to do on the cast level or the function level? For the case discussed, it would be sufficient to have a way to mark a particular function signature as preferred in cases of ambiguity, and that would be a lot less likely to have side effects. Mind you, fixing the cast in general would fix far more annoying cases, but I also see it as something which would be very hard to get correct ... -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Fabien, My experience at trying to pass minor patches shows that the basic behavior is conservatism. Maybe this is necessary to the stability of the project, but this is really annoying for pretty small changes on side tools, and I think that it tends to over-conservatism and ampers good will. You have to argue a lot about trivial things. My ratio for passing very minor patches on pgbench for instance is 1:3 or worst, 1 unit programming and testing versus 3 units writing mails to argue about this and that. Maybe the ratio is better for big important patches. Your patch is quite representative, 1 line of trivial code, a few lines of tests and docs, and how many lines and time at writing mails? This is, personally, the *entire* reason I've been arguing for this patch. I see the arguments against it as being a case of unnecessary conservatism, and cynically realize that if a well-known major contributor had proposed it, the patch would be committed already. Our project has a serious, chronic problem with giving new patch-submitters a bad experience, and this patch is a good example of that. The ultimate result is that people go off to contribute to other projects where submissions are easier and the rules for what gets accepted are relatively transparent. Personally, I don't care about pg_sleep(interval) really. But I do care that a minor contributor be able to submit and have accepted a patch of clear general usability, and not get it rejected on the basis of it might break something for someone even though we don't have a clear idea who. Now, I do think the argument of we don't really need pg_sleep(interval) because it's trivial to do yourself has some merit, and that would be a good reason to argue acceptance or not. However, to date that has not been the topic of discussion. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 12:45 PM, Josh Berkus j...@agliodbs.com wrote: Obviously, the implicit casts are not for PostgreSQL and would be rightly rejected here, but I am not sure that the ability to prefer one function or operator over others in an overloading situation is such a bad idea. So far, our internal testing seems to show that it works well and doesn't break things. Hmmm. Is this better to do on the cast level or the function level? For the case discussed, it would be sufficient to have a way to mark a particular function signature as preferred in cases of ambiguity, and that would be a lot less likely to have side effects. Mind you, fixing the cast in general would fix far more annoying cases, but I also see it as something which would be very hard to get correct ... This is of course to some degree a matter of opinion. The ankle bone is connected to the leg bone, and the leg bone is connected to the knee bone, and all that. It's very legitimate to think that we can change the system in a variety of places to fix any given problem. But if you're asking my opinion, I think doing it on the function level is a whole lot better and easier to get right. A flag like the one I mentioned here can be set for one particular function with the absolute certainty that behavior will not change for any function with some other name. That type of surety is pretty much impossible to get with casts. It's also not clear to me that the rules are logically related to the input data type. We might prefer to choose pg_sleep(double) over pg_sleep(interval) on backwards-compatibility grounds, but some other function might be in the opposite situation. You can't really get a lot of fine-grained control with casting here. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 10:01 AM, Robert Haas wrote: But if you're asking my opinion, I think doing it on the function level is a whole lot better and easier to get right. A flag like the one I mentioned here can be set for one particular function with the absolute certainty that behavior will not change for any function with some other name. That type of surety is pretty much impossible to get with casts. The other argument for doing it at the function level is that we could then expose it to users, who could use it to manage their own overloaded functions. We would NOT want to encourage users to mess with cast precedence, because it would be impossible for them to achieve their desired result that way. On the other hand, prioritization at the function level likely wouldn't help us with operators at all, because there the cast has to be chosen before we choose a function. So if we pursued the function route, then we'd eventually want to add a preferred flag for operators too. Which would be a lot more trouble, because it would affect the planner, but at least that would be a seperate step. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 12:59 PM, Josh Berkus j...@agliodbs.com wrote:
Now, I do think the argument of we don't really need pg_sleep(interval)
because it's trivial to do yourself has some merit, and that would be a
good reason to argue acceptance or not. However, to date that has not
been the topic of discussion.
I've made that exact argument several times on this thread. For example:
http://www.postgresql.org/message-id/CA+TgmobKneq=f9e8tzywg6haotzxozpvjqh14mpb9f+xlv6...@mail.gmail.com
I've been focusing on the backward compatibility issue mostly BECAUSE
I don't think the feature has much incremental value. If logical
replication or parallel query required breaking pg_sleep('42'), I
wouldn't be objecting. I'm sorry if that wasn't clear, and I further
apologize if you think I'm being too hard on a new patch submitter.
--
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 1:10 PM, Josh Berkus j...@agliodbs.com wrote: On 10/17/2013 10:01 AM, Robert Haas wrote: But if you're asking my opinion, I think doing it on the function level is a whole lot better and easier to get right. A flag like the one I mentioned here can be set for one particular function with the absolute certainty that behavior will not change for any function with some other name. That type of surety is pretty much impossible to get with casts. The other argument for doing it at the function level is that we could then expose it to users, who could use it to manage their own overloaded functions. We would NOT want to encourage users to mess with cast precedence, because it would be impossible for them to achieve their desired result that way. On the other hand, prioritization at the function level likely wouldn't help us with operators at all, because there the cast has to be chosen before we choose a function. So if we pursued the function route, then we'd eventually want to add a preferred flag for operators too. Which would be a lot more trouble, because it would affect the planner, but at least that would be a seperate step. Actually the operator resolution code is very much parallel to the function resolution code. I am quite sure in Advanced Server we only needed to add proisweak to handle both cases; unless I'm quite mistaken, we did not add oprisweak. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Josh Berkus j...@agliodbs.com wrote:
Our project has a serious, chronic problem with giving new
patch-submitters a bad experience, and this patch is a good
example of that.
Perhaps; but it has also been an example of the benefits of having
tight review. IMO, pg_sleep_for() and pg_sleep_until() are better
than the initial proposal. For one thing, since each accepts a
specific type, it allows for cleaner syntax. These are not only
unambiguous, they are easier to code and read than what was
originally proposed:
select pg_sleep_for('10 minutes');
select pg_sleep_until('tomorrow 05:00');
--
Kevin Grittner
EDB: 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 08:36 PM, Kevin Grittner wrote:
Josh Berkus j...@agliodbs.com wrote:
Our project has a serious, chronic problem with giving new
patch-submitters a bad experience, and this patch is a good
example of that.
Perhaps; but it has also been an example of the benefits of having
tight review.
FWIW, I agree. I have been impressed by the rigorous review process of
this project ever since I started following it.
IMO, pg_sleep_for() and pg_sleep_until() are better
than the initial proposal.
I agree here, as well. I was quite pleased with myself when I thought
of it, and I would not have thought of it had it not been for all the
pushback I received. Whether it goes in or not (I hope it does), I am
happy with the process.
For one thing, since each accepts a
specific type, it allows for cleaner syntax. These are not only
unambiguous, they are easier to code and read than what was
originally proposed:
select pg_sleep_for('10 minutes');
select pg_sleep_until('tomorrow 05:00');
These are pretty much exactly the examples I put in my documentation.
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 06:59 PM, Josh Berkus wrote: Our project has a serious, chronic problem with giving new patch-submitters a bad experience, and this patch is a good example of that. The ultimate result is that people go off to contribute to other projects where submissions are easier and the rules for what gets accepted are relatively transparent. That may be true, but it depends on the contributor. I would much rather be told that my contribution is not up to snuff than what happened on another project I recently tried to contribute to for the first time. A parser refactoring broke my code. I reported it and it was promptly fixed. When the fix came up for review, I said it needed a regression test to prevent it from happening again and I was told by the author that such a test would be flimsy and it went on to be committed (by that same guy) without one. I'm undecided whether I'll be contributing there any further. The rigor here makes me want to try and try again. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hello Vik,
Yes, I understand you are trying to help, and I appreciate it! My
opinion, and that of others as well from the original thread, is that
this patch should either go in as is and break that one case, or not go
in at all. I'm fine with either (although clearly I would prefer it
went in otherwise I wouldn't have written the patch).
I see this is marked as rejected in the commitfest app, but I don't see
any note about who did it or why. I don't believe there is consensus
for rejection on this list. In fact I think the opposite is true.
May we have an explanation please from the person who rejected this
without comment?
I did it, on the basis that you stated that you prefered not adding
pg_sleep(TEXT) to answer Robert Haas concern about preserving
pg_sleep('10') current functionality, and that no other solution was
suggested to tackle this issue.
If I'm mistaken, feel free to change the state back to what is
appropriate.
My actual opinion is that breaking pg_sleep('10') is no big deal, but I'm
nobody here, and Robert is somebody, so I think that his concern must be
addressed.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/16/2013 10:57 AM, Fabien COELHO wrote:
Hello Vik,
I see this is marked as rejected in the commitfest app, but I don't see
any note about who did it or why. I don't believe there is consensus
for rejection on this list. In fact I think the opposite is true.
May we have an explanation please from the person who rejected this
without comment?
I did it, on the basis that you stated that you prefered not adding
pg_sleep(TEXT) to answer Robert Haas concern about preserving
pg_sleep('10') current functionality, and that no other solution was
suggested to tackle this issue.
The suggested solution is to ignore the issue.
If I'm mistaken, feel free to change the state back to what is
appropriate.
I'm not really sure what the proper workflow is for marking a patch as
rejected, actually. I wouldn't mind some clarification on this from the
CFM or somebody.
In the meantime I've set it to Ready for Committer because in my mind
there is a consensus for it (see below) and you don't appear to have
anything more to say about the patch except for the do-we/don't-we issue.
My actual opinion is that breaking pg_sleep('10') is no big deal, but
I'm nobody here, and Robert is somebody, so I think that his concern
must be addressed.
Tom Lane is somebody, too, and his opinion is to break it or reject it
although he refrains from picking a side[1]. Josh Berkus and Stephen
Frost are both somebodies and they are on the break it side[2][3].
Peter Eisentraut gave no opinion at all but did say that Robert's
argument was not very good. I am for it because I wrote the patch, and
you seem not to care. So the way I see it we have:
For: Josh, Stephen, me
Against: Robert
Neutral: Tom, you
I don't know if that's enough of a consensus to commit it, but I do
think it's not nearly enough of a consensus to reject it.
[1] http://www.postgresql.org/message-id/16727.1376697147%40sss.pgh.pa.us
[2] http://www.postgresql.org/message-id/520ec584.3050...@agliodbs.com
[3]
http://www.postgresql.org/message-id/20130820013033.gu2...@tamriel.snowman.net
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
* Vik Fearing (vik.fear...@dalibo.com) wrote: I don't know if that's enough of a consensus to commit it, but I do think it's not nearly enough of a consensus to reject it. This is actually a problem that I think we have today- the expectation that *everyone* has to shoot down an idea for it to be rejected, but one individual saying oh, that's a good idea means it must be committed. That's not how it works and there's no notion of pending further discussion in the CF; imv that equates to returned with feedback. Marking this patch as 'Ready for Committer' when multiple committers have already commented on it doesn't strike me as moving things forward either. As it relates to this specific patch for this CF, I'd go with 'Returned with Feedback' and encourage you to consider the arguments for and against, and perhaps try to find existing usage which would break due to this change and consider the impact of changing it. For example, what do the various languages and DB abstraction layers do today? Would users of Hibernate likely be impacted or no? What about PDO? Personally, I'm still on-board with the change in general, but it'd really help to know that normal/obvious usage through various languages won't be busted by the change. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 2013-10-16 11:18:55 -0400, Stephen Frost wrote: This is actually a problem that I think we have today- the expectation that *everyone* has to shoot down an idea for it to be rejected, but one individual saying oh, that's a good idea means it must be committed. But neither does a single objection mean it cannot get committed. I don't see either scenario being present here though. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Wed, Oct 16, 2013 at 11:18 AM, Stephen Frost sfr...@snowman.net wrote:
* Vik Fearing (vik.fear...@dalibo.com) wrote:
I don't know if that's enough of a consensus to commit it, but I do
think it's not nearly enough of a consensus to reject it.
This is actually a problem that I think we have today- the expectation
that *everyone* has to shoot down an idea for it to be rejected, but
one individual saying oh, that's a good idea means it must be
committed.
That's not how it works and there's no notion of pending further
discussion in the CF; imv that equates to returned with feedback.
Marking this patch as 'Ready for Committer' when multiple committers
have already commented on it doesn't strike me as moving things forward
either.
As it relates to this specific patch for this CF, I'd go with 'Returned
with Feedback' and encourage you to consider the arguments for and
against, and perhaps try to find existing usage which would break due to
this change and consider the impact of changing it. For example, what
do the various languages and DB abstraction layers do today? Would
users of Hibernate likely be impacted or no? What about PDO?
Personally, I'm still on-board with the change in general, but it'd
really help to know that normal/obvious usage through various languages
won't be busted by the change.
I generally agree, although I'm not as sanguine as you about the
overall prospects for the patch. The bottom line is that there are
cases, like pg_sleep('42') that will be broken by this, and if you fix
those by some hack there will be other cases that break instead.
Recall what happened with pg_size_pretty(), which did not turn out
nearly as well as I thought it would, though I advocated for it at the
time. There's just no such thing as a free lunch: we *can't* change
the API for functions that have been around for many releases without
causing some pain for users that are depending on those functions.
Now, of course, sometimes it's worth it. We can and should change
things when there's enough benefit to justify the pain that comes from
breaking backward compatibility. But I don't see that as being the
case here. Anyone who actually wants this in their environment can
add the overloaded function in their environment with a one-line SQL
function: pg_sleep(interval) as proposed here is just
pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())).
Considering how easy that is, I don't see why we need to have it in
core.
In general, I'm a big fan of composibility as a design principle. If
you have a function that does A and a function that does B, it's
reasonable to say that people should use them together, rather than
providing a third function that does AB. Otherwise, you just end up
with too many functions. Of course, there are exceptions: if A and B
are almost always done together, a convenience function may indeed be
warranted. But I don't see how you can argue that here; there are
doubtless many existing users of pg_sleep(double) that are perfectly
happy with the existing behavior.
--
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/16/2013 08:18 AM, Stephen Frost wrote: As it relates to this specific patch for this CF, I'd go with 'Returned with Feedback' and encourage you to consider the arguments for and against, and perhaps try to find existing usage which would break due to this change and consider the impact of changing it. For example, what do the various languages and DB abstraction layers do today? Would users of Hibernate likely be impacted or no? What about PDO? Personally, I'm still on-board with the change in general, but it'd really help to know that normal/obvious usage through various languages won't be busted by the change. I'm fairly sure that the only language likely to be impacted chronically is PHP. Java, Ruby and Python, as a rule, properly data-type their parameters; PHP is the only language I know of where developers *and frameworks* chronically pass everything as a string. IIRC, the primary complainers when we removed a bunch of implicit casts in 8.3 were PHP devs. One thing to keep in mind, though, is that few developers actually use pg_sleep(), and I'd be surprised to find in in any canned web framework. Generally, if a web developer is going to sleep, they do it in the application. So we're really only talking about stored procedure writers here. And, as a rule, we can expect stored procedure writers to not gratuitously use strings in place of integers. We generally don't bounce PLpgSQL features which break unsupported syntax in PLpgSQL, since we expect people who write functions to have a better knowledge of SQL syntax than people who don't. I think pg_sleep(interval) falls under the same test. So I'd vote to accept it. Also, as Tom pointed out, at some point we have to either say we really support overloading or we don't. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus j...@agliodbs.com wrote: Also, as Tom pointed out, at some point we have to either say we really support overloading or we don't. We clearly do support overloading. I don't think that's at issue. But as we all know, using it can cause formerly unambiguous queries to become ambiguous and stop working. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert Haas robertmh...@gmail.com wrote: Anyone who actually wants this in their environment can add the overloaded function in their environment with a one-line SQL function: pg_sleep(interval) as proposed here is just pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())). You're making it sound way harder than it is. Why not just: create function my_sleep(delay interval) returns void language sql as 'select pg_sleep(extract(epoch from $1))'; ... or, of course, named to match the existing function. -- Kevin Grittner EDB: 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/16/13 2:40 PM, Robert Haas wrote:
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus j...@agliodbs.com wrote:
Also, as Tom pointed out, at some point we have to either say we really
support overloading or we don't.
We clearly do support overloading. I don't think that's at issue.
But as we all know, using it can cause formerly unambiguous queries to
become ambiguous and stop working.
But that's not really what this is. It's one thing to be wary about
adding foo(bigint, int, smallint) when there are already three other
permutations in the system. (At least in other languages, compilers
will give you warnings or errors when this creates an ambiguity, so
there's no guessing.) But the problem here is that if there already is a
foo(type1)
then the proposal to add
foo(type2)
can always be shot down by
But this will break foo('type1val').
That can't be in the spirit of overloading.
The only way to fix this is that at the time when you add foo(type1) you
need to prevent people from being able to call foo('type1val') and
instead require the full syntax foo(type1 'type1val'). The only way to
do that, I think, is to add some other foo(type3) at the same time.
There is just something wrong with that.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Wed, Oct 16, 2013 at 4:34 PM, Kevin Grittner kgri...@ymail.com wrote: Robert Haas robertmh...@gmail.com wrote: Anyone who actually wants this in their environment can add the overloaded function in their environment with a one-line SQL function: pg_sleep(interval) as proposed here is just pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())). You're making it sound way harder than it is. Why not just: create function my_sleep(delay interval) returns void language sql as 'select pg_sleep(extract(epoch from $1))'; ... or, of course, named to match the existing function. Because that might or might not do the right thing if the interval is 1 month. -- 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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 09/30/2013 01:47 PM, Vik Fearing wrote: Yes, I understand you are trying to help, and I appreciate it! My opinion, and that of others as well from the original thread, is that this patch should either go in as is and break that one case, or not go in at all. I'm fine with either (although clearly I would prefer it went in otherwise I wouldn't have written the patch). I see this is marked as rejected in the commitfest app, but I don't see any note about who did it or why. I don't believe there is consensus for rejection on this list. In fact I think the opposite is true. May we have an explanation please from the person who rejected this without comment? -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 09/22/2013 02:17 PM, Fabien COELHO wrote:
Hello,
There is no pg_sleep(text) function and the cast is unknown-double
precision.
My mistake.
As I understand it, pg_sleep('12') currently works and would not
anymore once your patch is applied. That is the concern raised by
Robert Haas.
That is correct.
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.
That was just a suggestion, and I was trying to help. ISTM that if
Robert's concern is not addressed one way or another, you will just
get rejected on this basis.
Yes, I understand you are trying to help, and I appreciate it! My
opinion, and that of others as well from the original thread, is that
this patch should either go in as is and break that one case, or not go
in at all. I'm fine with either (although clearly I would prefer it
went in otherwise I wouldn't have written the patch).
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
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 *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 7586,7599 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT /indexterm para ! The following function is available to delay execution of the server process: synopsis pg_sleep(replaceableseconds/replaceable) /synopsis functionpg_sleep/function makes the current session's process ! sleep until replaceableseconds/replaceable seconds have elapsed. replaceableseconds/replaceable is a value of type typedouble precision/, so fractional-second delays can be specified. For example: --- 7586,7601 /indexterm para ! The following functions are available to delay execution of the server process: synopsis pg_sleep(replaceableseconds/replaceable) + pg_sleep(replaceableinterval/replaceable) /synopsis functionpg_sleep/function makes the current session's process ! sleep until replaceableseconds/replaceable seconds (or the specified ! replaceableinterval/replaceable) have elapsed. replaceableseconds/replaceable is a value of type typedouble precision/, so fractional-second delays can be specified. For example: *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0 --- 3017,3024 DESCR(list all files in a directory); DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ )); DESCR(sleep for the specified time in seconds); + DATA(insert OID = 3179 ( pg_sleep PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())) _null_ _null_ _null_ )); + DESCR(sleep for the specified interval); DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 25 16 _null_ _null_ _null_ _null_ booltext _null_ _null_ _null_ )); DESCR(convert boolean to text); *** a/src/test/regress/expected/stats.out --- b/src/test/regress/expected/stats.out *** *** 18,24 SET enable_indexscan TO on; SET enable_indexonlyscan TO off; -- wait to let any prior tests finish dumping out stats; -- else our messages might get lost due to contention ! SELECT pg_sleep(2.0); pg_sleep -- --- 18,24 SET enable_indexonlyscan TO off; -- wait to let any prior tests finish dumping out stats; -- else our messages might get lost due to contention ! SELECT pg_sleep(interval '2 seconds'); pg_sleep -- *** a/src/test/regress/sql/stats.sql --- b/src/test/regress/sql/stats.sql *** *** 16,22 SET enable_indexonlyscan TO off; -- wait to let any prior tests finish dumping out stats; -- else our messages might get lost due to contention ! SELECT pg_sleep(2.0); -- save counters CREATE TEMP TABLE prevstats AS --- 16,22 -- wait to let any prior tests finish dumping out stats; -- else our messages might get lost due to contention ! SELECT pg_sleep(interval '2 seconds'); -- save counters CREATE TEMP TABLE prevstats AS -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hello,
There is no pg_sleep(text) function and the cast is unknown-double
precision.
My mistake.
As I understand it, pg_sleep('12') currently works and would not anymore
once your patch is applied. That is the concern raised by Robert Haas.
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.
That was just a suggestion, and I was trying to help. ISTM that if
Robert's concern is not addressed one way or another, you will just get
rejected on this basis.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Fri, Sep 20, 2013 at 7:59 AM, Fabien COELHO coe...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
* Josh Berkus (j...@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 signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Fri, Aug 16, 2013 at 4:16 PM, Peter Eisentraut pete...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Aug 8, 2013 at 7:52 AM, Vik Fearing vik.fear...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark st...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 8/16/13 3:35 PM, Robert Haas wrote:
On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark st...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Peter Eisentraut pete...@gmx.net writes:
On 8/16/13 3:35 PM, Robert Haas wrote:
On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark st...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
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
Re: [HACKERS] [PATCH] pg_sleep(interval)
Josh Berkus j...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 08/16/2013 05:15 PM, Tom Lane wrote: Josh Berkus j...@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
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Aug 8, 2013 at 1:52 PM, Vik Fearing vik.fear...@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
[HACKERS] [PATCH] pg_sleep(interval)
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. *** a/doc/src/sgml/func.sgml --- b/doc/src/sgml/func.sgml *** *** 7537,7550 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT /indexterm para ! The following function is available to delay execution of the server process: synopsis pg_sleep(replaceableseconds/replaceable) /synopsis functionpg_sleep/function makes the current session's process ! sleep until replaceableseconds/replaceable seconds have elapsed. replaceableseconds/replaceable is a value of type typedouble precision/, so fractional-second delays can be specified. For example: --- 7537,7552 /indexterm para ! The following functions are available to delay execution of the server process: synopsis pg_sleep(replaceableseconds/replaceable) + pg_sleep(replaceableinterval/replaceable) /synopsis functionpg_sleep/function makes the current session's process ! sleep until replaceableseconds/replaceable seconds (or the specified ! replaceableinterval/replaceable) have elapsed. replaceableseconds/replaceable is a value of type typedouble precision/, so fractional-second delays can be specified. For example: *** a/src/include/catalog/pg_proc.h --- b/src/include/catalog/pg_proc.h *** *** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0 --- 3017,3024 DESCR(list all files in a directory); DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 701 _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ )); DESCR(sleep for the specified time in seconds); + DATA(insert OID = 3179 ( pg_sleep PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 1186 _null_ _null_ _null_ _null_ select pg_sleep(extract(epoch from now() + $1) - extract(epoch from now())) _null_ _null_ _null_ )); + DESCR(sleep for the specified interval); DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 25 16 _null_ _null_ _null_ _null_ booltext _null_ _null_ _null_ )); DESCR(convert boolean to text); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [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
