groovy

Include transaction closure in standard api

Details

  • Type: New Feature New Feature
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.5.6
  • Fix Version/s: 1.6.2, 1.7-beta-1
  • Component/s: SQL processing
  • Labels:
    None
  • Number of attachments :
    1

Description

When doing a set of write operations sometimes a statement-set should follow transaction semantics. Thus a closure like withTransaction{} inside groovy.sql.Sql class would be very handy.

For discussion see also http://www.nabble.com/groovy.sql.Sql-and-doing-SQL-statements-in-one-transaction-td2594039.html#a20562605

Activity

Hide
Rick Reumann added a comment -

I'd love to see this included by default also, and it would be great if this usage as shown in the email above (which I found through Google) was added to the groovy sql docs. (In the meantime can someone add this good closure example to the docs?)

Show
Rick Reumann added a comment - I'd love to see this included by default also, and it would be great if this usage as shown in the email above (which I found through Google) was added to the groovy sql docs. (In the meantime can someone add this good closure example to the docs?)
Hide
Guillaume Laforge added a comment -

The online documentation is backed by Confluence, so you can freely help us improve the documentation, add more examples, etc.
You just need to register on the site.
Such documentation contributions are very much appreciated.

Also, you can submit a patch for this feature, and we'll review it and perhaps include it in an upcoming version of Groovy.

Show
Guillaume Laforge added a comment - The online documentation is backed by Confluence, so you can freely help us improve the documentation, add more examples, etc. You just need to register on the site. Such documentation contributions are very much appreciated. Also, you can submit a patch for this feature, and we'll review it and perhaps include it in an upcoming version of Groovy.
Hide
Paul King added a comment -

Potential patch attached. I haven't had time to test it yet.

Show
Paul King added a comment - Potential patch attached. I haven't had time to test it yet.
Hide
Paul King added a comment -

added to trunk, pending merge onto 1_6_X branch

Show
Paul King added a comment - added to trunk, pending merge onto 1_6_X branch
Hide
Paul King added a comment -

merged

Show
Paul King added a comment - merged
Hide
Rick Reumann added a comment -

I can't seem to get this to work if I'm using a DataSource. I'll end up with
INFO: Caught exception closing connection: java.sql.SQLException: Connection is closed.
java.sql.SQLException: Connection is closed.

I tried different ways to get it to work and the only thing that seemed to work for me was using what Guillaume posted a while back (I just modified it to call a closure.) It's not as nice though (or maybe it's ok), because it also requires passing back a new Sql instance. When I tried this not passing back a new sql instance, the transaction never rolled back.

So what I have is:

Sql.metaClass.useTransaction = { Closure closure ->
java.sql.Connection conn = null
try { conn = createConnection() conn.autoCommit = false Sql sql2 = new Sql( conn ) closure(sql2) conn.commit() } catch (e) { if(conn != null) conn.rollback() e.printStackTrace() throw e } finally {
if ( conn != null ) {
conn.autoCommit = true
if ( dataSource ) { conn.close() }
}
}
}

which of course requires our closure to use an instance of the Sql passed back in the closure. I'm sure there is probably a way to get the patched version to work using a datasource but you guys would know best.

Not sure if this should a new issue?

Show
Rick Reumann added a comment - I can't seem to get this to work if I'm using a DataSource. I'll end up with INFO: Caught exception closing connection: java.sql.SQLException: Connection is closed. java.sql.SQLException: Connection is closed. I tried different ways to get it to work and the only thing that seemed to work for me was using what Guillaume posted a while back (I just modified it to call a closure.) It's not as nice though (or maybe it's ok), because it also requires passing back a new Sql instance. When I tried this not passing back a new sql instance, the transaction never rolled back. So what I have is: Sql.metaClass.useTransaction = { Closure closure -> java.sql.Connection conn = null try { conn = createConnection() conn.autoCommit = false Sql sql2 = new Sql( conn ) closure(sql2) conn.commit() } catch (e) { if(conn != null) conn.rollback() e.printStackTrace() throw e } finally { if ( conn != null ) { conn.autoCommit = true if ( dataSource ) { conn.close() } } } } which of course requires our closure to use an instance of the Sql passed back in the closure. I'm sure there is probably a way to get the patched version to work using a datasource but you guys would know best. Not sure if this should a new issue?
Hide
Paul King added a comment -

Can you try again using trunk? I hope the problem has gone away with the latest changes.

Show
Paul King added a comment - Can you try again using trunk? I hope the problem has gone away with the latest changes.
Hide
Paul King added a comment -

Now in the 1_6_X branch as well. If you have any feedback before the 1.6.3 release it would be most appreciated. You might want to look at the relevant test cases to see the usage patterns.

Show
Paul King added a comment - Now in the 1_6_X branch as well. If you have any feedback before the 1.6.3 release it would be most appreciated. You might want to look at the relevant test cases to see the usage patterns.
Hide
Rick Reumann added a comment -

Paul, I finally got back to looking at this stuff, I have a quick question...

I'm a bit confused on the logic within the withTransaction method. What is the purpose of setting cacheConnection = false in the finally block only to then turn around and set it again with the savedCacheConnection. Does closeResources do something where you want to make sure the cacheConnection is always false first before calling closeResources().. that's the only reason I could see to setting it to false first, but it doesn't look closeResources would care? Was just curious.

(I also have another question related to the class being Thread safe which I'll ask on the list since it could be useful in case anyone else is searching the forums.)

Thanks again.

Show
Rick Reumann added a comment - Paul, I finally got back to looking at this stuff, I have a quick question... I'm a bit confused on the logic within the withTransaction method. What is the purpose of setting cacheConnection = false in the finally block only to then turn around and set it again with the savedCacheConnection. Does closeResources do something where you want to make sure the cacheConnection is always false first before calling closeResources().. that's the only reason I could see to setting it to false first, but it doesn't look closeResources would care? Was just curious. (I also have another question related to the class being Thread safe which I'll ask on the list since it could be useful in case anyone else is searching the forums.) Thanks again.

People

Vote (4)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: