Skip to content

Commit e8f6087

Browse files
committed
gh-161837: Fix SystemError in sqlite3 when a collation fails during fetch
A collation callback cannot report an error to SQLite, so the Python exception stays set. A later error in the same step can make sqlite3_step() fail. Then pysqlite_cursor_iternext() calls set_error_from_db() with the exception still set. This causes a SystemError. In debug builds, an assertion fails. _pysqlite_query_execute() already prints or clears the pending exception before it raises the DB-API exception. Move that code into the new helper clear_callback_error() and use it in both functions.
1 parent eb21dea commit e8f6087

3 files changed

Lines changed: 67 additions & 9 deletions

File tree

‎Lib/test/test_sqlite3/test_hooks.py‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import sqlite3 as sqlite
2525
import unittest
2626

27-
from test.support import import_helper
27+
from test.support import captured_stderr, import_helper
2828
from test.support.os_helper import TESTFN, unlink
2929

3030
from .util import memory_database, cx_limit, with_tracebacks
@@ -106,6 +106,49 @@ def mycoll(x, y):
106106
self.assertEqual(result, [('c',), ('b',), ('a',)],
107107
msg="the expected order was not returned")
108108

109+
def test_collation_error_then_step_failure_while_fetching(self):
110+
# gh-161837: A collation can set an exception. Then a later error can
111+
# make the same sqlite3_step() fail. The fetch must raise
112+
# OperationalError for that failure, not SystemError.
113+
def mycoll(x, y):
114+
if x == "boom":
115+
raise KeyError("collation raised")
116+
return (x > y) - (x < y)
117+
self.con.create_collation("mycoll", mycoll)
118+
self.con.execute("create table t(x, y)")
119+
# abs() of the smallest 64-bit integer causes "integer overflow".
120+
self.con.executemany("insert into t values (?, ?)",
121+
[(1, "a"), (-2**63, "boom")])
122+
sql = """
123+
select case when y = 'q' collate mycoll then abs(x) else 0 end
124+
from t order by rowid
125+
"""
126+
127+
def fetch_with_fetchone():
128+
cur = self.con.execute(sql)
129+
self.assertEqual(cur.fetchone(), (0,))
130+
cur.fetchone()
131+
132+
def fetch_by_iteration():
133+
list(self.con.execute(sql))
134+
135+
for fetch in fetch_with_fetchone, fetch_by_iteration:
136+
for tracebacks in False, True:
137+
with self.subTest(fetch=fetch.__name__, tracebacks=tracebacks):
138+
sqlite.enable_callback_tracebacks(tracebacks)
139+
try:
140+
with captured_stderr() as err:
141+
with self.assertRaisesRegex(sqlite.OperationalError,
142+
"integer overflow"):
143+
fetch()
144+
finally:
145+
sqlite.enable_callback_tracebacks(False)
146+
if tracebacks:
147+
self.assertIn("KeyError: 'collation raised'",
148+
err.getvalue())
149+
else:
150+
self.assertEqual(err.getvalue(), "")
151+
109152
def test_collation_register_twice(self):
110153
"""
111154
Register two different collation functions under the same name.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a :exc:`SystemError` in :mod:`sqlite3`. The error occurs when a
2+
collation callback raises an exception and the same step then fails during a
3+
fetch. In debug builds, an assertion fails. Now the module raises
4+
:exc:`~sqlite3.OperationalError` for the step failure, as
5+
:meth:`~sqlite3.Cursor.execute` does.

‎Modules/_sqlite/cursor.c‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,22 @@ stmt_step(sqlite3_stmt *statement)
553553
return rc;
554554
}
555555

556+
/* A user-defined callback (for example, a collation) can set an exception
557+
* during a failed sqlite3_step(). Report that exception, then clear it.
558+
* The caller can then raise the DB-API exception for the failure. */
559+
static void
560+
clear_callback_error(pysqlite_state *state)
561+
{
562+
if (PyErr_Occurred()) {
563+
if (state->enable_callback_tracebacks) {
564+
PyErr_Print();
565+
}
566+
else {
567+
PyErr_Clear();
568+
}
569+
}
570+
}
571+
556572
static int
557573
bind_param(pysqlite_state *state, pysqlite_Statement *self, int pos,
558574
PyObject *parameter)
@@ -911,14 +927,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
911927

912928
rc = stmt_step(self->statement->st);
913929
if (rc != SQLITE_DONE && rc != SQLITE_ROW) {
914-
if (PyErr_Occurred()) {
915-
/* there was an error that occurred in a user-defined callback */
916-
if (state->enable_callback_tracebacks) {
917-
PyErr_Print();
918-
} else {
919-
PyErr_Clear();
920-
}
921-
}
930+
clear_callback_error(state);
922931
set_error_from_db(state, self->connection->db);
923932
goto error;
924933
}
@@ -1177,6 +1186,7 @@ pysqlite_cursor_iternext(PyObject *op)
11771186
}
11781187
}
11791188
else if (rc != SQLITE_ROW) {
1189+
clear_callback_error(self->connection->state);
11801190
rc = set_error_from_db(self->connection->state, self->connection->db);
11811191
int reset_rc = stmt_reset(self->statement);
11821192
Py_CLEAR(self->statement);

0 commit comments

Comments
 (0)