You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
andersblomdellcontrollthse commented on Nov 23, 2010
andersblomdellcontrollthsemannequin
MannequinAuthor
More actions
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.
andersblomdellcontrollthse commented on Nov 24, 2010
andersblomdellcontrollthsemannequin
MannequinAuthor
More actions
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()
printc.fetchall()
is:
[(0,), (0,)]
which is not what I would expect with a primary key...
fviard commented on Dec 23, 2011
fviardmannequin
Mannequin
More actions
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:
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;')
whileTrue:
print"-- One iter --"results=cursor.fetchmany()
ifnotresults:
breakforresultinresults:
printstr(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?
andersblomdellcontrollthse commented on Feb 16, 2012
andersblomdellcontrollthsemannequin
MannequinAuthor
More actions
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 :-)
paulmelis commented on Apr 22, 2013
paulmelismannequin
Mannequin
More actions
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.
paulmelis commented on May 1, 2013
paulmelismannequin
Mannequin
More actions
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.0sqlite: 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.0sqlite: 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:
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.
Activity
andersblomdellcontrollthse commentedon Nov 23, 2010
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()
The program works with 2.6.2
ned-deily commentedon Nov 23, 2010
Also fails with 3.2 as in 2.7 and works in 3.1 as in 2.6.
andersblomdellcontrollthse commentedon Nov 24, 2010
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:
is:
which is not what I would expect with a primary key...
fviard commentedon Dec 23, 2011
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:
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,))
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?
andersblomdellcontrollthse commentedon Feb 16, 2012
Would be really nice to not have to fix this in ech new release :-)
paulmelis commentedon Apr 22, 2013
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.
paulmelis commentedon May 1, 2013
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:
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:
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
Gian-CarloPascutto commentedon Mar 6, 2015
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.
python-dev commentedon Aug 26, 2016
New changeset 81f614dd8136 by Berker Peksag in branch '3.5':
Issue bpo-10513: Fix a regression in Connection.commit()
https://hg.python.org/cpython/rev/81f614dd8136
New changeset 685f32972c11 by Berker Peksag in branch 'default':
Issue bpo-10513: Merge from 3.5
https://hg.python.org/cpython/rev/685f32972c11
python-dev commentedon Aug 26, 2016
New changeset 030e100f048a by Berker Peksag in branch '2.7':
Issue bpo-10513: Fix a regression in Connection.commit()
https://hg.python.org/cpython/rev/030e100f048a
berkerpeksag commentedon Aug 26, 2016
This is now fixed in 2.7 and 3.5+. Thank you all for your patience!
python-dev commentedon Jan 12, 2017
New changeset dd13098a5dc2 by Benjamin Peterson in branch '2.7':
revert 030e100f048a (bpo-29006, bpo-10513)
https://hg.python.org/cpython/rev/dd13098a5dc2
python-dev commentedon Jan 16, 2017
New changeset 74a68d86569c by Benjamin Peterson in branch '2.7':
revert dd13098a5dc2 (bpo-29006, bpo-10513)
https://hg.python.org/cpython/rev/74a68d86569c
erlend-aasland commentedon May 7, 2021
I cannot reproduce this with Python 3.8, 3.9, nor 3.10 (macOS builds from python.org). Suggesting to close this.
erlend-aasland commentedon May 15, 2021
Closing as fixed. If someone disagrees; please re-open.
Remove the retry hack for issue 5432
Remove the retry hack for issue 5432