coleifer / pysqlite3

SQLite3 DB-API 2.0 driver from Python 3, packaged separately, with improvements
zlib License
188 stars 53 forks source link

Autocommit behaves differently from sqlite3 #58

Closed simonw closed 1 year ago

simonw commented 1 year ago

I was getting the error "cannot change into wal mode from within a transaction" when trying to switch to WAL mode using this code:

db.execute('PRAGMA journal_mode=wal;')

Upon further experimentation, it appears that setting db.isolation_level = None fixes this - but this fix is not necessary for sqlite3. Here's a console session illustrating the difference between the two:

>>> import sqlite3
>>> db = sqlite3.connect("/tmp/1.db")
>>> db.isolation_level
''
>>> db.execute('PRAGMA journal_mode;').fetchall()
[('delete',)]
>>> db.execute('PRAGMA journal_mode=wal;')
<sqlite3.Cursor object at 0x7f272ef748c0>
>>> db.execute('PRAGMA journal_mode;').fetchall()
[('wal',)]

Now the same sequence with pysqlite3:

>>> import pysqlite3
>>> db2 = pysqlite3.connect("/tmp/2.db2")
>>> db2.isolation_level
''
>>> db2.execute('PRAGMA journal_mode;').fetchall()
[('delete',)]
>>> db2.execute('PRAGMA journal_mode=wal;')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
pysqlite3.dbapi2.OperationalError: cannot change into wal mode from within a transaction
>>> db2.isolation_level = None
>>> db2.execute('PRAGMA journal_mode=wal;')
<pysqlite3.dbapi2.Cursor object at 0x7f272ef45d80>
>>> db2.execute('PRAGMA journal_mode;').fetchall()
[('wal',)]

Is this a deliberate difference, or accidental?

coleifer commented 1 year ago

I dislike the way standard lib does it. Instead of inspecting each SQL string (which is error prone), we use sqlite3_stmt_readonly. Unfortunately, to maintain db-api compatibility, all database-modifying stuff must be run in a transaction. The only way around this is to set the isolation level to None at present for setting up wal-mode. Happily, this is something you only need to do once before writing anything to your database, and after that it will be persisted.

Note: see next comment below, I've added an exclusion for pragma, so going forwards this should not implicitly require a transaction.

coleifer commented 1 year ago

I could probably extend the checks, actually, to include pragma now that I think of it.

3769367a202535a110d008bcb046eb08f26871cc

coleifer commented 1 year ago

The following is now working properly for me:

from pysqlite3 import dbapi2 as sqlite3
conn = sqlite3.connect('/tmp/t1.db')
conn.execute('pragma journal_mode=wal;')
erlend-aasland commented 1 year ago

FWIW, I gave up using sqlite3_stmt_readonly; using that alone would have been a backwards incompatible change, and the added complexity of trying to remain backwards compatible was not worth the effort. For the stdlib, I'm afraid we'll have to keep the current behaviour, even though I do not like it myself.

coleifer commented 1 year ago

@erlend-aasland I agree with your decision and think it's absolutely correct to be more compatible. Since I had the opportunity (and a lot less users) Ive taken a few liberties.

At the end of the day, @simonw you could always look into apsw as well.

erlend-aasland commented 1 year ago

Yeah, not living in the stdlib gives you a lot of freedom :)

Come to think of it, I may get away with it in the upcoming autocommit feature; when that feature is used (autocommit is True), sqlite3_stmt_readonly is used to check for DML statements; if in legacy mode, use the old behaviour. IMO, there should be room to tweak that feature in the beta phase.