Re: [JDBC] Support for JDBC setQueryTimeout, et al.

From: "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov>
To: Rados*aw Smogura <rsmogura(at)softperience(dot)eu>
Cc: "David Fetter" <david(at)fetter(dot)org>, "PG Hackers" <pgsql-hackers(at)postgresql(dot)org>, "PostgreSQL JDBC List" <pgsql-jdbc(at)postgresql(dot)org>
Subject: Re: [JDBC] Support for JDBC setQueryTimeout, et al.
Date: 2010-10-13 18:27:01
Message-ID: 4CB5B3A50200002500036939@gw.wicourts.gov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-jdbc

Rados*aw Smogura<rsmogura(at)softperience(dot)eu> wrote:

> I updated patch to latets CVS version, I didn't have time to
> remove some trashes from it.
>
> If something will be wrong with patch, give a feedback.

I skimmed it and agree that it is the right general approach --
using java.util.Timer to trigger the cancel method. It doesn't
confuse the function of the setQueryTimeout method of the JDBC
driver with the statement_timeout GUC of PostgreSQL, which strike me
as no more or less similar to each other than the brakes on my car
are to a highway guardrail -- both are designed to stop something,
but under different circumstances.

I certainly can't fault you for lack of testing, since about
two-thirds of the patch is testing classes. I didn't see any need
to include the last two classes (ByteConverter and
InfiniteTimerTask); can you explain why those are in there?

That said, some of the details of the implementation gave me pause
-- there seem to be rather more moving parts and more places to
check things than the overall complexity of the problem would seem
to warrant; however, it's entirely possible that on closer review
I'll find that they were necessary for reasons which escape me on a
quick scan of the code.

If you could add this to the open CommitFest, I'll be glad to review
it (if nobody beats me to it):

https://commitfest.postgresql.org/action/commitfest_view/open

-Kevin

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2010-10-13 18:32:22 Why do we have a database specification in .pgpass?
Previous Message Alex Hunsaker 2010-10-13 18:17:19 Re: Slow count(*) again...

Browse pgsql-jdbc by date

  From Date Subject
Next Message Robert Haas 2010-10-14 01:01:06 Re: [JDBC] Support for JDBC setQueryTimeout, et al.
Previous Message Radosław Smogura 2010-10-13 17:17:48 Re: [JDBC] Support for JDBC setQueryTimeout, et al.