Re: [JDBC] Support for JDBC setQueryTimeout, et al.
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Kevin Grittner <Kevin.Grittner@wicourts.gov>
Cc: "Rados*aw Smogura" <rsmogura@softperience.eu>, David Fetter <david@fetter.org>, PG Hackers <pgsql-hackers@postgresql.org>, PostgreSQL JDBC List <pgsql-jdbc@postgresql.org>
Date: 2010-10-14T01:01:06Z
Lists: pgsql-hackers
On Wed, Oct 13, 2010 at 2:27 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Rados*aw Smogura<rsmogura@softperience.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 Is this a JDBC patch or a PG patch? Are we tracking JDBC patches using the CF app? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company