Jochen Theodorou wrote:
Hi,

> No new issue needed I would say. Also the code looks good, there are
> just a few small things...

I have updated the code while taking into account your remarks

> (1) there is no documentation on the code

You would say documentation on new methods or long, textual and contextual documentation ?
I have put javadoc documentation.

> (2) maybe connectionPrepareStatement, createCache, getCachedStatement
> should be private?

Done

> (3) Aaron Digulla mentioned, that we need to
> ensure that no wrong connection will be used... does the code ensure
> this?

You're thinking about dataSource's connection ? If we're doing statement caching, current connection must not be closed and keep as long as you have to.
So we have just to consider dataSource's connection as passed as parameter connection (see property groovy.sql.Sql.useConnection)
When caching is over, connection is normally closed.

> (4) the usage of the word persistent is probably a bit
> misleading... at
> last without any documentation
> (6) if holdOn is there, do we need a public API for persistent?
> (7) we probably need to find a better name for holdOn ;) How about
> cacheStaments for example?

Ok ok :) holdOn is replaced par cacheStatement(Closure closure) and setPersistent(Boolean b) is repalced by
cacheStatement(Boolean b).

cacheStatement(Closure closure) is usefull for request nesting in the closure-look way
Sql.cacheStatement {
 sql.eachRow(...) {
   sql.eachRow(...)
 }
}

cacheStatement(Boolean b) is usefull for just avoid wasting resource

sql.cacheStatement = true
sql.eachRow(...) {}
...
sql.eachRow(...) {}
...
sql.cacheStatement = false

> (5) holdOn does log the exception and swallow it...this
> should not happen

Taken into account and corrected.


> (8) maybe this should be a general feature
> and we should use
> a property
> that enables caching for all code in Sql.java

In fact this feature now allows to cache statements, preparedStatement and not to waste connection.

I get now Sql patch and unit tests. I put them in the same issue ?


--
Marc DeXeT
CNRS - DSI
Pôle Expertises
<http://www.dsi.cnrs.fr>


---------------------------------------------------------------------
To unsubscribe from this list, please visit:

  http://xircles.codehaus.org/manage_email