Re: [PATCH] pg_sleep(interval)

On Wed, Oct 16, 2013 at 11:18 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Vik Fearing (vik(dot)fearing(at)dalibo(dot)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