38

Say you have the following code:

Connection conn;
try
{
   conn = ... // get connection
   conn.setAutoCommit(false);

   ... // Do some modification queries and logic

   conn.commit()
} catch(SQLException e)
{
    conn.rollback() // Do we need this?
    conn.close()
}

In this code, if there is an exception, is it better style to just close the connection (since autocommit is off), or to explicitly roll back and then close the connection? There are no save points.

I feel that it might make sense to add the rollback call because:

1) Someone, in the future, might add save points but forget to add the rollback

2) It improves readability

3) It shouldn't cost anything, right ?

But obviously, none of these is particularly compelling. Any standard practice?

Note: I'm aware of the need to do a repeat try/catch on closing and rollback. I actually have a middleware that abstracts the database access and takes care of that, but I was wondering whether adding it was superfluous.

Uri
  • 88,451
  • 51
  • 221
  • 321
  • 1
    Good question. I'm interesting in the answer to your 3rd item because we used to always do rollback before returning a connection, but at one point our DBA noticed this and claimed, quite religiously, that rollbacks were expensive. I didn't get a good motivation, but we gave in and basically made sure rollback() was only called if it was needed (which was easy because it was only one place in the code that was affected). – waxwing Jul 01 '10 at 20:56
  • Could anybody answer this question with Java 7 try-with-resource? Would this simplify the code that has to be written? (Related question, but does not explain how to use try-with-resource in combination with autoCommit: http://stackoverflow.com/questions/8066501/how-should-i-use-try-with-resources-with-jdbc) – user1202136 May 05 '15 at 14:34

2 Answers2

46

The normal idiom is the following:

public void executeSomeQuery() throws SQLException {
    try (Connection connection = dataSource.getConnection()) {
        connection.setAutoCommit(false);

        try (PreparedStatement statement = connection.prepareStatement(SOME_SQL)) {
            // Fire transactional queries here.

            connection.commit();
        } catch (SQLException e) {
            connection.rollback();
            throw e;
        }
    }
}

Note that Java 7's try-with-resources statement always implicitly calls close() on the resource when the try block finishes, as if it happens in finally.

Calling rollback() is also mandatory when it concerns a pooled connection. Namely, it will reset the transactional state of the connection. The close() of a pooled connection won't do that, only the commit() and rollback() will do that. Not calling rollback() may lead that the next lease of the pooled connection will still have the (successful) queries of the previous transaction in its memory.

See also javadoc of Connection#close() (emphasis not mine):

It is strongly recommended that an application explicitly commits or rolls back an active transaction prior to calling the close method. If the close method is called and there is an active transaction, the results are implementation-defined.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 1
    Ah. Thank you. I was not aware of the behavior in the case of pooled connections (which we used). Most of our legacy code doesn't use it while most of mine does. This is a compelling reason. Is this documented anywhere? – Uri Jul 01 '10 at 21:10
  • 3
    The [`rollback()` javadoc](http://java.sun.com/javase/6/docs/api/java/sql/Connection.html#rollback%28%29) mentions: *releases any database locks currently held by this `Connection` object*. You can read it as "transactional locks". Combine this with the fact that connections from a connection pool will not *actually* be closed but just returned for pool for reuse in other queries. Not commiting/rollbacking it will risk them to use the same lock cq. transaction. – BalusC Jul 01 '10 at 21:14
  • @BalusC : Thanks for that:) I have experienced this. What happens if I close a connection obtained from a connection pool? – Ashwin Jun 06 '12 at 10:06
  • FWIW, *commons-dbcp2* has [`BasicDataSource.setRollbackOnReturn(boolean)`](http://commons.apache.org/proper/commons-dbcp/api-2.1/org/apache/commons/dbcp2/BasicDataSource.html#setRollbackOnReturn-boolean-), which is `true` by default, for automatically rolling back transactions on `Connection.close()`. – antak May 15 '15 at 08:17
  • 2
    When you set autoCommit(false), do you need to set it back to true, or does it just affect the connection temporarily? – EdgeCase Nov 19 '15 at 16:42
  • @EdgeCase I think you would need to reset autocommit explicitly. – shmosel Jan 12 '16 at 04:42
  • @BalusC Can I use connection.prepareStatement(SOME_SQL) without try-with-resource? I think PreparedStatement will be closed by Connection#close. – guest Sep 27 '18 at 17:29
2

Closing should rollback because it will not commit when the resources are release, but it's good to have your error handling be specific, so if you want to rollback on an exception, do that. Then you can do your cleanup in a finally{} block. The rollback() occurs only on error, in which case your commit() is not successful or was not even reached.

Connection conn = null;
try {
    conn = ...

    ...
    conn.commit();
}
catch (SQLException e) {
    if (conn != null) {
        conn.rollback();
    }
}
finally {
    if (conn != null) {
        conn.close();
    }
}
jbindel
  • 5,535
  • 2
  • 25
  • 38
  • 1
    Beware that `close()` and `rollback()` can (unfortunately) also throw an `SQLException` which you must handle (see BalusC's answer). – Jesper Jul 01 '10 at 21:07
  • That's true, and the compiler will make sure you handle those, possibly in the calling code, but that can be rather ugly. – jbindel Jul 01 '10 at 21:46
  • 1
    Using jdk >=7 , you can use the try-with-resources syntax to use the auto close behavior on Closable objets. – Nrj Jul 02 '15 at 13:00