Java: commit vs rollback vs nothing when semantics is unchanged?

Question

Ok, I know the difference between commit and rollback and what these operations are supposed to do. However, I am not certain what to do in cases where I can achieve the same behavior when using commit(), rollback() and/or do nothing.

For instance, let's say I have the following code which executes a query without writing to db: I am working on an application which communicates with SQLite database.

try {
  doSomeQuery()
  // b) success
} catch (SQLException e) {
  // a) failed (because of exception)
}

Or even more interesting, consider the following code, which deletes a single row:

try {
  if (deleteById(2))
    // a) delete successful (1 row deleted)
  else
    // b) delete unsuccessful (0 row deleted, no errors)
} catch (SQLException e) {
  // c) delete failed (because of an error (possibly due to constraint violation in DB))
}

Observe that from a semantic standpoint, doing commit or rollback in cases b) and c) result in the same behavior.

Generally, there are several choices to do inside each case (a, b, c):

• commit
• rollback
• do nothing

Are there any guidelines or performance benefits of choosing a particular operation? What is the right way?

Note: Assume that auto-commit is disabled.

Answer 1

What you are saying depends upon the code being called, does it return a flag for you to test against, or does it exclusively throw exceptions if something goes wrong?

API throws exceptions but also returns a boolean (true|false):

This situation occurs a lot, and it makes it difficult for the calling code to handle both conditions, as you pointed out in your OP. The one thing you can do in this situation is:

// Initialize a var we can test against later
//  Lol, didn't realize this was for java, please excuse the var
//  initialization, as it's demonstrative only
$queryStatus = false;

try {
    if (deleteById(2)) {
        $queryStatus = true;
    } else {
        // I can do a few things here
        // Skip it, because the $queryStatus was initialized as false, so 
        //  nothing changes
        // Or throw an exception to be caught
        throw new SQLException('Delete failed');
    }        
} catch (SQLException $e) {
    // This can also be skipped, because the $queryStatus was initialized as 
    //  false, however you may want to do some logging
    error_log($e->getMessage());    
}

// Because we have a converged variable which covers both cases where the API
//  may return a bool, or throw an exception we can test the value and determine
//  whether rollback or commit
if (true === $queryStatus) {
    commit();
} else {
    rollback();
}

API exclusively throws exceptions (no return value):

No problem. We can assume that if no exception was caught, the operation completed without error and we can add rollback() / commit() within the try/catch block.

try {
    deleteById(2);

    // Because the API dev had a good grasp of error handling by using
    //  exceptions, we can also do some other calls
    updateById(7);

    insertByName('Chargers rule');

    // No exception thrown from above API calls? sweet, lets commit
    commit();

} catch (SQLException $e) {

    // Oops exception was thrown from API, lets roll back
    rollback();
}

API does not throw any exceptions, only returns a bool (true|false):

This goes back to old school error handling/checking

if (deleteById(2)) {
    commit();
} else {
    rollback();
}

If you have multiple queries making up the transaction, you can borrow the single var idea from scenario #1:

$queryStatus = true;

if (!deleteById(2)) {
    $queryStatus = false;
} 

if (!updateById(7)) {
    $queryStatus = false;
} 

...

if (true === $queryStatus) {
    commit();
} else {
    rollback();
}

"Note: Assume that auto-commit is disabled."

• Once you disable auto-commit, you are telling the RDBMS that you are taking control of commits from now until auto-commit is re-enabled, so IMO, it's good practice to either rollback or commit the transaction versus leaving any queries in limbo.

Answer 2

If it is just a select, I wouldn't open a transaction and therefore, there is nothing to do in any case. Probably you already know is there is an update/insert since you have already passed the parameters. The case where you intend to do a manipulation is more interesting. The case where it deletes is clear you want to commit; if there is an exception you should rollback to keep the DB consistent since something failed and there is not a lot you can do. If the delete failed because there was nothing to delete I'd commit for 3 reasons:

• Semantically, it seems more correct since the operation was technically successful and performed as specified.

• It is more future proof so if somebody adds more code to the transaction they won't be surprised with it is rolling back because one delete just didn't do anything (they would expect on exception that the transaction is rolled back)

• When there is an operation to do, commit is faster but in this case I don't think it matters.

Answer 3

As others stated in their answers, it's not a matter of performance (for the equivalent cases you described), which I believe is negligible, but a matter of maintainability, and this is ever so important!

In order for your code to be nicely maintainable, I suggest (no matter what) to always commit at the very bottom of your try block and to always close your Connection in your finally block. In the finally block you also should rollback if there are uncommitted transactions (meaning that you didn't reach the commit at the end of the try block).

This example shows what I believe is the best practice (miantainability-wise):

public boolean example()
{
    Connection conn;
    ...
    try
    {
        ...
        //Do your SQL magic (that can throw exceptions)
        ...
        conn.commit();
        return true;
    }
    catch(...)
    {
        ...
        return false;
    }
    finally
    {//Close statements, connections, etc
        ...
        closeConn(conn);
    }
}

public static void closeConn(Connection conn)
{
    if (conn != null)
        if (!conn.isClosed())
        {
            if (!conn.getAutoCommit())
                conn.rollback();//If we need to close() but there are uncommitted transacitons (meaning there have been problems)
            conn.close();
            conn = null;
        }
}

Answer 4

I've asked myself the exact same question. In the end I went for a solution where I always commited successful transactions and always rollbacked non-successful transactions regardles of it having any effect. This simplified lot of code and made it clearer and more easy to read.

It did not have any major performance problems in the application I worked in which used NHibernate + SQLite on .net. Your milage may vary.

Answer 5

Any non-trivial application will have operations requiring multiple SQL statements to complete. Any failure happening after the first SQL statement and before the last SQL statement will cause data to be inconsistent.

Transactions are designed to make multiple-statement operations as atomic as the single-statement operations you are currently working with.