omnilib / aiosqlite

asyncio bridge to the standard sqlite3 module
https://aiosqlite.omnilib.dev
MIT License
1.22k stars 94 forks source link

commit parity with sqlite3 doesn't exist with `conn.commit()` #286

Open startakovsky opened 8 months ago

startakovsky commented 8 months ago

Description

I was reading this and learned that the context manager for the sqlite3 library automatically handles commits but not closes. I am referring to this line from the Python documentation: # Connection object used as context manager only commits or rollbacks transactions

Seems like the situation is not the same with aiosqlite. That you need to commit.

In other words the first example below should work, but it doesn't.

Doesn't work in an analogous fashion with sqlite3.

async with aiosqlite.connect(DB_FILEPATH) as conn:
        await conn.execute(
            '''
            UPDATE .... 
            ''', 
            (
                ...values
            )
        )
conn.close()

Current code requires an explicit conn.commit()

async with aiosqlite.connect(DB_FILEPATH) as conn:
        await conn.execute(
            '''
            UPDATE .... 
            ''', 
            (
                ...values
            )
        )
       await conn.commit()
conn.close()
startakovsky commented 8 months ago

Anyway, so I believe this is a bug and my request would be to do modify the context manager for the aiosqlite.connection object. I am not going to (attempt) to submit a PR unless I know we're in agreement or if I'm wrong etc. or any special caveats as to what makes this hard to do.

teobouvard commented 7 months ago

This issue was already raised 3 years ago in #110, and it did not receive any answers either.

amyreese commented 7 months ago

Happy to review a PR that adds similar behavior to the connection object. The part that concerns me is this:

If there is no open transaction upon leaving the body of the with statement, or if autocommit is True, the context manager does nothing.

I’m not sure how that’s handled in sqlite, or if that requires special checks, but would like to make sure that nuance is matched here as well.

Edit: and of course, I would want any PR to maintain the existing aiosqlite behavior of automatically closing the connection when the context ends, in addition to any new behavior to automatically commit open transactions.