execute("begin immediate") throwing OperationalError

[fschulze]

BPO	28518
Nosy	@jaraco, @ned-deily, @bitdancer, @socketpair, @berkerpeksag, @serhiy-storchaka, @animalize, @zhangyangyu, @palaviv, @fschulze
PRs	bpo-28518: Start a transaction implicitly before a DML statement #245 [3.6] bpo-28518: Start a transaction implicitly before a DML statement (#245) #318 [Do Not Merge] Sample of CPython life with blurb. #703 [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	28518.patch sqlite-explicit-begin.patch sqlite-explicit-begin2.patch sqlite-ddl-dml.patch sqlite-ddl-dml-2.patch sqlite-ddl-dml-3.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-02-26.16:09:49.161>
created_at = <Date 2016-10-24.11:05:18.283>
labels = ['3.7', 'type-bug', 'library']
title = 'execute("begin immediate") throwing OperationalError'
updated_at = <Date 2017-03-31.16:36:23.953>
user = 'https://github.com/fschulze'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:23.953>
actor = 'dstufft'
assignee = 'none'
closed = True
closed_date = <Date 2017-02-26.16:09:49.161>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2016-10-24.11:05:18.283>
creator = 'fschulze'
dependencies = []
files = ['45639', '45644', '45923', '46079', '46354', '46397']
hgrepos = []
issue_num = 28518
keywords = ['patch']
message_count = 35.0
messages = ['279301', '279305', '279808', '281626', '281633', '281638', '281639', '281691', '281700', '281704', '281709', '281724', '281735', '281738', '281739', '282463', '283345', '283398', '284268', '284373', '284392', '284397', '284538', '284585', '285618', '285893', '286084', '286087', '286122', '286300', '286308', '286525', '288599', '290380', '290391']
nosy_count = 14.0
nosy_names = ['ghaering', 'jaraco', 'ned.deily', 'r.david.murray', 'socketpair', 'berker.peksag', 'serhiy.storchaka', 'malin', 'xiang.zhang', 'palaviv', 'Big Stone', 'fschulze', 'callidomus', 'C\xc3\xa9dric Bellegarde']
pr_nums = ['245', '318', '703', '552']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28518'
versions = ['Python 3.6', 'Python 3.7']

[fschulze]

Using:

conn = sqlite3.connect(':memory:', isolation_level='IMMEDIATE')
conn.execute('begin immediate')

Throws:

sqlite3.OperationalError: cannot start a transaction within a transaction

This didn't happen in previous versions and the conn.in_transaction attribute is False right before the call to execute, so this situation doesn't seem to be detectable upfront for backward compatibility.

[zhangyangyu]

Looks like commit 284676cf2ac8 in bpo-10740 introduces this. Haven't read through the thread yet.

[palaviv]

I looked into this error and I think the problem is the sqlite3_stmt_readonly check in _pysqlite_query_execute (cursor.c line 517):

    if (self->connection->begin_statement && !sqlite3_stmt_readonly(self->statement->st) && !self->statement->is_ddl) {
        if (sqlite3_get_autocommit(self->connection->db)) {
            result = _pysqlite_connection_begin(self->connection);
            if (!result) {
                goto error;
            }
            Py_DECREF(result);
        }
    }

As you can see we check if the current statement is not readonly and if so we start a transaction. The problem is that sqlite3_stmt_readonly return false for the "begin immediate" statement. When this happens we open a transaction with _pysqlite_connection_begin and then executing the "begin immediate".

I think this is a error in sqlite as the documentation says:
"ransaction control statements such as BEGIN, COMMIT, ROLLBACK, SAVEPOINT, and RELEASE cause sqlite3_stmt_readonly() to return true,"
(https://www.sqlite.org/c3ref/stmt_readonly.html)

[fschulze]

How do we proceed from here? The Python 3.6.0rc1 is looming. If this is a bug in sqlite, then we need a workaround. We can't wait for a fix in sqlite, because it's not bundled with Python and without the fix Python 3.6 breaks with older sqlite versions. I'd like to help, but don't know how.

[bitdancer]

I'm going to mark this as a release blocker because it is a regression. Ned may or may not agree. What is needed is for someone to propose a patch.

[serhiy-storchaka]

Sorry, I'm not experienced with sqlite3, but in what circumstances conn.execute('begin immediate') makes sense? Could you please provide a larger example where it is needed?

[ned-deily]

Also, if there is indeed a suspected error in the underlying library, has the issue been reported to the SQLite project?

[palaviv]

I searched in the sqlite tickets from something related to this issue and I couldn't find anything. I wanted to check the sqlite-users mailing list to see if anyone had already reported it but there is a problem in gmane archives.

I tried to find a good way to solve this problem but I couldn't. I am adding a patch that should solve this problem but it is for sure not a good way :(.

[berkerpeksag]

I haven't had a time to investigate this yet, but I don't think there is an issue with SQLite itself.

Note that pysqlite behaves the same way since version 2.8.0 and I couldn't find a similar report in their issue tracker.

Like Serhiy, I'm also a little puzzled with the reproducer. I think if you want to start an explicit transaction, the preferred way would be to set isolation_level to None. Otherwise, the sqlite3 module will start one implicitly for you (or perhaps I still don't understand PEP-249's transaction model 100% correctly :))

(If we decide that this is not a regression, we should add a test case to cover this use case.)

[fschulze]

Ok, I reread https://docs.python.org/3/library/sqlite3.html#controlling-transactions several times now. I think it's confusing.

I want explicit transaction handling, but isolation_level = None turns on "autocommit mode". Unfortunately I don't have suggestions on how to improve the documentation.

For us, isolation_level = None and the explicit begin immediate seems to be the correct way and it works with all Python versions, including 3.6.0.

So, I would say this is "not a bug" in Python, but my understanding of the sqlite3 module is flawed. The documentation could be better, but I don't know how to make it better.

[berkerpeksag]

There is a patch in bpo-8145 to improve that part of the documentation. It would be great if you could take a look at it :)