Re: [HACKERS] [PATCH] pg_sleep(interval)

2014-02-03 Thread Robert Haas
On Sun, Feb 2, 2014 at 4:50 AM, Julien Rouhaud 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 u

Re: [HACKERS] [PATCH] pg_sleep(interval)

2014-02-02 Thread Julien Rouhaud
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2014-01-30 Thread Vik Fearing
On 01/30/2014 09:48 PM, Robert Haas wrote: > On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing wrote: >> On 10/17/2013 02:42 PM, Robert Haas wrote: >>> On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing wrote: On 10/17/2013 10:03 AM, Fabien COELHO wrote: > My guess is that it won't be committed if

Re: [HACKERS] [PATCH] pg_sleep(interval)

2014-01-30 Thread Robert Haas
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing wrote: > On 10/17/2013 02:42 PM, Robert Haas wrote: >> On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing 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 co

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-21 Thread Stephen Frost
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 "f

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-21 Thread Robert Haas
On Fri, Oct 18, 2013 at 7:21 PM, Jim Nasby 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 inste

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-18 Thread Jim Nasby
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 wh

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-18 Thread Jim Nasby
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 absolut

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-18 Thread Josh Berkus
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 bette

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Vik Fearing
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Vik Fearing
On 10/17/2013 08:36 PM, Kevin Grittner wrote: > 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. > Perhaps; but it has also been an example of the benefits of having > tight review.

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Kevin Grittner
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. 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 t

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Thu, Oct 17, 2013 at 1: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 funct

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Thu, Oct 17, 2013 at 12:59 PM, Josh Berkus 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 di

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Josh Berkus
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 c

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Thu, Oct 17, 2013 at 12:45 PM, Josh Berkus 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 i

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Josh Berkus
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Josh Berkus
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 wel

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Peter Eisentraut
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Thu, Oct 17, 2013 at 9:51 AM, Alvaro Herrera 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, >> EnterpriseD

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Alvaro Herrera
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 >

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Fabien COELHO
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 longe

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Vik Fearing
On 10/17/2013 02:42 PM, Robert Haas wrote: > On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Vik Fearing
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Robert Haas
On Wed, Oct 16, 2013 at 5:16 PM, Peter Eisentraut wrote: > On 10/16/13 2:40 PM, Robert Haas wrote: >> On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus 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 ov

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-17 Thread Fabien COELHO
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 t

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 4:34 PM, Kevin Grittner wrote: > Robert Haas 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Peter Eisentraut
On 10/16/13 2:40 PM, Robert Haas wrote: > On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus 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,

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Kevin Grittner
Robert Haas 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 sou

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus 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 t

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Josh Berkus
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 impac

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Robert Haas
On Wed, Oct 16, 2013 at 11:18 AM, Stephen Frost 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Andres Freund
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 singl

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Stephen Frost
* 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Vik Fearing
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-16 Thread Fabien COELHO
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 i

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-10-15 Thread Vik Fearing
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 (altho

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-30 Thread Vik Fearing
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 > R

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-22 Thread Fabien COELHO
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)" cleanl

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-22 Thread Vik Fearing
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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-20 Thread Fabien COELHO
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_sle

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-20 Thread Robert Haas
On Fri, Sep 20, 2013 at 7:59 AM, Fabien COELHO 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 tha

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-09-20 Thread Fabien COELHO
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 s

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-23 Thread Fabien COELHO
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 p

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-23 Thread Peter Eisentraut
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 th

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-19 Thread Stephen Frost
* 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 decisi

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-18 Thread Fabien COELHO
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.o

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-17 Thread Robert Haas
On Fri, Aug 16, 2013 at 4:16 PM, Peter Eisentraut 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 P

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Josh Berkus
On 08/16/2013 05:15 PM, Tom Lane wrote: > Josh Berkus 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 tha

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Tom Lane
Josh Berkus 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 fac

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Josh Berkus
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 th

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Tom Lane
Peter Eisentraut writes: > On 8/16/13 3:35 PM, Robert Haas wrote: >> On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Peter Eisentraut
On 8/16/13 3:35 PM, Robert Haas wrote: > On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark 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(doubl

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Robert Haas
On Fri, Aug 16, 2013 at 2:57 PM, Greg Stark 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 $

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-16 Thread Greg Stark
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)

2013-08-16 Thread Robert Haas
On Thu, Aug 8, 2013 at 7:52 AM, Vik Fearing 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

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-11 Thread Magnus Hagander
On Thu, Aug 8, 2013 at 1:52 PM, Vik Fearing 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 com

Re: [HACKERS] [PATCH] pg_sleep(interval)

2013-08-08 Thread Fabien COELHO
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 ch

[HACKERS] [PATCH] pg_sleep(interval)

2013-08-08 Thread Vik Fearing
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 acco