classification
Title: sqlite3.InterfaceError after commit
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Gian-Carlo Pascutto, anders.blomdell@control.lth.se, berker.peksag, fviard, ghaering, paulmelis, python-dev
Priority: normal Keywords: patch

Created on 2010-11-23 12:35 by anders.blomdell@control.lth.se, last changed 2017-01-16 08:07 by python-dev.

Files
File name Uploaded Description Edit
sqlite_bug.py anders.blomdell@control.lth.se, 2010-11-23 12:35 Simple program that crashes
sqlite_bug.py anders.blomdell@control.lth.se, 2010-11-24 10:08 Modified example
test_bug_pysql_for.py fviard, 2011-12-23 14:05 previous example + new testcase
bug10513.patch paulmelis, 2013-04-22 19:57 Patch
bug-binding_parameter_0.py paulmelis, 2013-05-01 08:57
Messages (13)
msg122213 - (view) Author: Anders Blomdell (anders.blomdell@control.lth.se) Date: 2010-11-23 12:35
With version 2.7 (and 2.7.1rc1), the following sequence (see attached test):
   
   c =  cursor.execute(' select k from t where k == ?;', (1,))
   conn.commit()
   r = c.fetchone()

Traceback (most recent call last):
  File "/bugs/sqlite_bug.py", line 22, in <module>
    c =  cursor.execute(' select k from t where k == ?;', (2,))
sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.

The program works with 2.6.2
msg122226 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2010-11-23 17:31
Also fails with 3.2 as in 2.7 and works in 3.1 as in 2.6.
msg122270 - (view) Author: Anders Blomdell (anders.blomdell@control.lth.se) Date: 2010-11-24 10:08
The culprit seems to be 'pysqlite_do_all_statements(self, ACTION_RESET, 0)' in pysqlite_connection_commit, which resets all active statements, but subsequent fetch/fetchall seems to trash the sqlite3 state in the statements. Removing the ACTION_RESET seems to bring back old behaviour (if it's the correct fix is, however, beyond me).

Slightly modified testprogram that shows more wierdness; output from:

    c =  cursor.execute(' select k from t where k == ?;', (0,))
    conn.commit()
    print c.fetchall()

is:

    [(0,), (0,)]

which is not what I would expect with a primary key...
msg150162 - (view) Author: Florent Viard (fviard) Date: 2011-12-23 14:05
Hi,

I encountered the same regression with python 2.7.
I added a new testcase (testcase2) with another effect of the same problem. It is a model of what does python-storm when I try to do:

for item in TableA.findall():
  TableB.new_item(name=item.name)
  connection.commit()

As you can see in the result:
-- One iter --
(0,)
-- One iter --
(1,)
-- One iter --
(0,)
-- One iter --
(1,)
-- One iter --
(2,)

Inside the for loop, there is a reset, but only one time. So entries returned by "for" are inconsistents as there could be some duplicated results.


After studying the code, I understood that this regression is caused by the addition of function call:
        pysqlite_do_all_statements(self, ACTION_RESET, 0);
in connection.c: in function: pysqlite_connection_commit

Removing this line, fixes the problem and bring back the old correct behavior.


For the explanation of my case, I understood that:
For the "statement" of my query, the "in_use" flag is first set to 1, the first fetchmany call ran, then the commit triggered the reset of all the statements that have  the in_use flag == 1 and set this flag to 0. So the next fetchmany call return results from the beginning again without care for the state of the "in_use" flag. Then, the next commit don't reset again the statement as its "in_use" flag is still == 0.

I think, that the first intention of this modification was more to use the following call:
pysqlite_do_all_statements(self, ACTION_RESET, 1);
to have the cursor->reset set and so, at next fetch call, triggering exception: "sqlite3.InterfaceError: Cursor needed to be reset because of commit/rollback and can no longer be fetched from."

But as for using it in rollback, I'm not sure that this behavior is really useful as sqlite3 looks to be able to correctly handle modifications and cursor return values.
As an example, the following test with the previous code :
    cursor = conn.cursor()
    cursorBis = conn.cursor()
    cursor.execute('create table first(id primary key);')
    for i in range(5):
        cursor.execute('insert or replace into first(id) values(?);', (i,))

    results = cursor.execute(' select id from first;')
    while True:
        print "-- One iter --"
        results = cursor.fetchmany()
        if not results:
            break
        for result in results:
            print str(result)
            cursorBis.execute(' delete from first where id == ?;', (3,))
        conn.commit()

That correctly printed:
-- One iter --
(0,)
-- One iter --
(1,)
-- One iter --
(2,)
-- One iter --
(4,)
-- One iter --


So my suggestion is to remove in "pysql_connection_commit" the call to :
pysqlite_do_all_statements(self, ACTION_RESET, 0);
to bring back the correct old behavior.

And also eventually to remove in "pysqlite_connection_rollback" the call to :
pysqlite_do_all_statements(self, ACTION_RESET, 1);


What do you think about it?
msg153508 - (view) Author: Anders Blomdell (anders.blomdell@control.lth.se) Date: 2012-02-16 22:35
> So my suggestion is to remove in "pysql_connection_commit" the call to :
> pysqlite_do_all_statements(self, ACTION_RESET, 0);
> to bring back the correct old behavior.
That's what I have been running for years, now...

> And also eventually to remove in "pysqlite_connection_rollback" the 
> call to :
> pysqlite_do_all_statements(self, ACTION_RESET, 1);
Have no opinion on this

Would be really nice to not have to fix this in ech new release :-)
msg187585 - (view) Author: Paul Melis (paulmelis) Date: 2013-04-22 19:57
Here's a patch that removes the
pysqlite_do_all_statements(self, ACTION_RESET, 0);
call.

It also adds the sqlite error code to one of the exceptions raised, as the error message is misleading in case the ACTION_RESET is left in (I forgot what sqlite error is actually raised in that case).

I've been using this patch for some while now with 2.7.3 and it fixes similar problems as noted in this thread.
msg188204 - (view) Author: Paul Melis (paulmelis) Date: 2013-05-01 08:57
Just a bit more info on the patch. When running stock Python 2.7.4 the attached test script bug-binding_parameter_0.py returns:

module: 2.6.0
sqlite: 3.7.9
Archives
Archives/2011
Archives/2012
Traceback (most recent call last):
  File "bug-binding_parameter_0.py", line 45, in <module>
    cur = dbconn.execute('select uidvalidity from folders where name=?', (folder,))
sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.

The error suggests a misuse of the sqlite3 API, but the actual SQLite error is masked. After altering _sqlite/statement.c to include the SQLite error code (as in the patch), we get:

module: 2.6.0
sqlite: 3.7.9
Archives
Archives/2011
Archives/2012
Traceback (most recent call last):
  File "bug-binding_parameter_0.py", line 45, in <module>
    cur = dbconn.execute('select uidvalidity from folders where name=?', (folder,))
sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type. (sqlite error 21)

Error 21 = SQLITE_MISUSE, suggesting something is definitely wrong in the way the SQLite API is used (for this test case). Commenting out the ACTION_RESET all works fine again:

module: 2.6.0
sqlite: 3.7.9
Archives
Archives/2011
Archives/2012
msg237402 - (view) Author: Gian-Carlo Pascutto (Gian-Carlo Pascutto) Date: 2015-03-06 23:40
I believe http://bugs.python.org/issue23129 is a dupe of this.

The patch here has been in "patch review" for 9 months. That seems fairly long for something that's a regression that potentially silently produces the wrong data.
msg273716 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-26 19:07
New changeset 81f614dd8136 by Berker Peksag in branch '3.5':
Issue #10513: Fix a regression in Connection.commit()
https://hg.python.org/cpython/rev/81f614dd8136

New changeset 685f32972c11 by Berker Peksag in branch 'default':
Issue #10513: Merge from 3.5
https://hg.python.org/cpython/rev/685f32972c11
msg273718 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-26 19:17
New changeset 030e100f048a by Berker Peksag in branch '2.7':
Issue #10513: Fix a regression in Connection.commit()
https://hg.python.org/cpython/rev/030e100f048a
msg273721 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-26 19:21
This is now fixed in 2.7 and 3.5+. Thank you all for your patience!
msg285293 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-12 07:40
New changeset dd13098a5dc2 by Benjamin Peterson in branch '2.7':
revert 030e100f048a (#29006, #10513)
https://hg.python.org/cpython/rev/dd13098a5dc2
msg285542 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-16 08:07
New changeset 74a68d86569c by Benjamin Peterson in branch '2.7':
revert dd13098a5dc2 (#29006, #10513)
https://hg.python.org/cpython/rev/74a68d86569c
History
Date User Action Args
2017-01-16 08:07:37python-devsetmessages: + msg285542
2017-01-12 07:41:11benjamin.petersonsetstatus: closed -> open
resolution: fixed ->
2017-01-12 07:40:47python-devsetmessages: + msg285293
2016-08-26 20:22:05berker.peksaglinkissue20562 superseder
2016-08-26 19:21:29berker.peksagsetstatus: open -> closed

versions: + Python 3.6, - Python 3.4
nosy: + berker.peksag

messages: + msg273721
resolution: fixed
stage: patch review -> resolved
2016-08-26 19:19:53berker.peksaglinkissue23129 superseder
2016-08-26 19:17:49python-devsetmessages: + msg273718
2016-08-26 19:07:39python-devsetnosy: + python-dev
messages: + msg273716
2015-03-06 23:40:25Gian-Carlo Pascuttosetnosy: + Gian-Carlo Pascutto
messages: + msg237402
2014-06-27 07:02:57Claudiu.Popasetstage: needs patch -> patch review
versions: + Python 3.5, - Python 3.2, Python 3.3
2013-05-01 08:57:05paulmelissetfiles: + bug-binding_parameter_0.py

messages: + msg188204
2013-04-22 20:50:11ned.deilysetnosy: - ned.deily
2013-04-22 19:57:31paulmelissetfiles: + bug10513.patch

nosy: + paulmelis
messages: + msg187585

keywords: + patch
2012-02-16 22:35:03anders.blomdell@control.lth.sesetmessages: + msg153508
2011-12-23 14:05:08fviardsetfiles: + test_bug_pysql_for.py
versions: + Python 3.3, Python 3.4
nosy: + fviard

messages: + msg150162
2010-11-24 10:08:11anders.blomdell@control.lth.sesetfiles: + sqlite_bug.py

messages: + msg122270
2010-11-23 17:31:44ned.deilysetversions: + Python 3.2
nosy: + ghaering, ned.deily

messages: + msg122226

type: crash -> behavior
stage: needs patch
2010-11-23 12:35:31anders.blomdell@control.lth.secreate