Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-[NSFNanoEngine isTransactionActive] works but misinterprets sqlite3_get_autocommit() #91

Closed
billgarrison opened this issue Apr 22, 2013 · 1 comment

Comments

@billgarrison
Copy link
Contributor

The current implementation of -[NSFNanoEngine isTransactionActive] treats the return value from sqlite3_get_autocommit() as an SQLite error code. But that doesn't match the documentation for the function.

- (BOOL)isTransactionActive
{
    sqlite3* myDB = self.sqlite;

    int status = sqlite3_get_autocommit(myDB);

    // Since we're operating with extended result code support, extract the bits
    // and obtain the regular result code
    // For more info check: http://www.sqlite.org/c3ref/c_ioerr_access.html

    status = [NSFNanoEngine NSFP_stripBitsFromExtendedResultCode:status];

    return (0 == status);
}

sqlite3_get_autocommit() is documented to return either 0 or a non-zero value, to be interpreted as a boolean. Stripping bits from the returned value is unnecessary and not strictly correct in this case.

A functionally equivalent, but more accurate implementation for this method would be:

- (BOOL)isTransactionActive
{
    /* 
    sqlite3_get_autocommit(sqlite3 *db) returns 0 when the given database connection 
    is in the middle of a manually-initiated transaction.
     */
    return (sqlite3_get_autocommit(self.sqlite) == 0);
}

The current implementation does work fine and returns a proper value. It was just confusing reading the code and comparing it against the SQLite documentation.

@tciuro
Copy link
Owner

tciuro commented Apr 25, 2013

Sure thing, looking at the documentation I don't see why this is needed. Thanks for reporting this.

@tciuro tciuro closed this as completed in 05089e8 May 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants