Gavin,
From the SQL-92 spec: "When a <commit statement> is executed, all constraints are effectively checked and, if any constraint is not satisfied, then an exception condition is raised and the transaction is terminated by an implicit <rollback statement>."
So even if classic JDBC examples have the commit statements in the try block, it is unnecessary to explicitly call rollback on commit failure. Therefore, I even more strongly support the "rollback-on-HibernateException-but-not-on-TransactionException" idiom, as it also matches JTA's model. Also supporting the "rollback-on-any-exception-including-commit-failure" idiom shouldn't be hard to do, but I definitely consider the "commit-OR-rollback" model cleaner.
So what about the following implementation of JDBCTransaction:
Code:
public void commit() throws HibernateException {
if (!begun) throw new TransactionException("Transaction not successfully started");
log.debug("commit");
if ( session.getFlushMode()!=FlushMode.NEVER ) session.flush();
try {
session.connection().commit();
committed = true;
session.afterTransactionCompletion(true);
}
catch (SQLException e) {
log.error("Commit failed", e);
session.afterTransactionCompletion(false);
throw new TransactionException("Commit failed with SQL exception: ", e);
}
finally {
toggleAutoCommit();
}
}
public void rollback() throws HibernateException {
if (!begun) throw new TransactionException("Transaction not successfully started");
log.debug("rollback");
try {
session.connection().rollback();
rolledBack=true;
}
catch (SQLException e) {
log.error("Rollback failed", e);
throw new TransactionException("Rollback failed with SQL exception: ", e);
}
finally {
session.afterTransactionCompletion(false);
toggleAutoCommit();
}
}
That implementation would leave both the connection and the session in a valid state after an actual commit attempt, in any case -- concerning autoCommit state and afterTransactionCompletion call. If flushing failed, a non-TransactionException HibernateException will get thrown, and no autoCommit change or afterTransactionCompletion call will have happened -- as application code is assumed to explicitly call rollback in that case.
If application code explicitly calls rollback on TransactionException too, an unnecessary rollback of an empty JDBC transaction will occur, and another reset of autoCommit state plus another afterTransactionCompletion call (none of these should matter). So effectively, the above implementation would support both idioms: Application developers are able to choose between "explicit-rollback-on-commit-failure" vs "commit-or-rollback" then. What do you think?
Juergen