In JDBC, when autocommit is false and no explicit savepoints have been set, is it good style or a waste to rollback?

Question

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.

Answer 1

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();
    }
}

Answer 2

The normal idiom is the following:

Connection connection;
try {
    connection = database.getConnection();
    connection.setAutoCommit(false);

    // Fire transactional queries here.

    connection.commit()
} catch (SQLException e) {
    // Rollback throws SQLException as well.
    if (connection != null) try { connection.rollback(); } catch (SQLException logOrIgnore) {}
    throw e; // You don't want to suppress the main exception.
} finally {
    // Closing should happen in finally.
    if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
}

Wrap if necessary the rollback() and close() in a public static void helper method to temper the verbosity.

Calling rollback() is mandatory when it concerns a pooled connection. It will namely 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 (succesful) queries of the previous transaction in its memory.