Skip to content

Commit b567b9d

Browse files
author
Erlend Egeberg Aasland
authored
bpo-45512: Simplify isolation_level handling in sqlite3 (GH-29053)
1 parent c399786 commit b567b9d

File tree

3 files changed

+84
-63
lines changed

3 files changed

+84
-63
lines changed

Modules/_sqlite/clinic/connection.c.h

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ preserve
55
static int
66
pysqlite_connection_init_impl(pysqlite_Connection *self,
77
const char *database, double timeout,
8-
int detect_types, PyObject *isolation_level,
8+
int detect_types, const char *isolation_level,
99
int check_same_thread, PyObject *factory,
1010
int cached_statements, int uri);
1111

@@ -22,7 +22,7 @@ pysqlite_connection_init(PyObject *self, PyObject *args, PyObject *kwargs)
2222
const char *database = NULL;
2323
double timeout = 5.0;
2424
int detect_types = 0;
25-
PyObject *isolation_level = NULL;
25+
const char *isolation_level = "";
2626
int check_same_thread = 1;
2727
PyObject *factory = (PyObject*)clinic_state()->ConnectionType;
2828
int cached_statements = 128;
@@ -63,7 +63,24 @@ pysqlite_connection_init(PyObject *self, PyObject *args, PyObject *kwargs)
6363
}
6464
}
6565
if (fastargs[3]) {
66-
isolation_level = fastargs[3];
66+
if (fastargs[3] == Py_None) {
67+
isolation_level = NULL;
68+
}
69+
else if (PyUnicode_Check(fastargs[3])) {
70+
Py_ssize_t isolation_level_length;
71+
isolation_level = PyUnicode_AsUTF8AndSize(fastargs[3], &isolation_level_length);
72+
if (isolation_level == NULL) {
73+
goto exit;
74+
}
75+
if (strlen(isolation_level) != (size_t)isolation_level_length) {
76+
PyErr_SetString(PyExc_ValueError, "embedded null character");
77+
goto exit;
78+
}
79+
}
80+
else {
81+
_PyArg_BadArgument("Connection", "argument 'isolation_level'", "str or None", fastargs[3]);
82+
goto exit;
83+
}
6784
if (!--noptargs) {
6885
goto skip_optional_pos;
6986
}
@@ -834,4 +851,4 @@ getlimit(pysqlite_Connection *self, PyObject *arg)
834851
#ifndef PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF
835852
#define PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF
836853
#endif /* !defined(PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF) */
837-
/*[clinic end generated code: output=d71bf16bef67878f input=a9049054013a1b77]*/
854+
/*[clinic end generated code: output=663b1e9e71128f19 input=a9049054013a1b77]*/

Modules/_sqlite/connection.c

Lines changed: 63 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ static const char * const begin_statements[] = {
7979
NULL
8080
};
8181

82-
static int pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* isolation_level, void *Py_UNUSED(ignored));
8382
static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self);
8483
static void free_callback_context(callback_context *ctx);
8584
static void set_callback_context(callback_context **ctx_pp,
@@ -107,6 +106,30 @@ new_statement_cache(pysqlite_Connection *self, int maxsize)
107106
return res;
108107
}
109108

109+
static inline const char *
110+
begin_stmt_to_isolation_level(const char *begin_stmt)
111+
{
112+
assert(begin_stmt != NULL);
113+
114+
// All begin statements start with "BEGIN "; add strlen("BEGIN ") to get
115+
// the isolation level.
116+
return begin_stmt + 6;
117+
}
118+
119+
static const char *
120+
get_begin_statement(const char *level)
121+
{
122+
assert(level != NULL);
123+
for (int i = 0; begin_statements[i] != NULL; i++) {
124+
const char *stmt = begin_statements[i];
125+
const char *candidate = begin_stmt_to_isolation_level(stmt);
126+
if (sqlite3_stricmp(level, candidate) == 0) {
127+
return begin_statements[i];
128+
}
129+
}
130+
return NULL;
131+
}
132+
110133
/*[python input]
111134
class FSConverter_converter(CConverter):
112135
type = "const char *"
@@ -124,7 +147,7 @@ _sqlite3.Connection.__init__ as pysqlite_connection_init
124147
database: FSConverter
125148
timeout: double = 5.0
126149
detect_types: int = 0
127-
isolation_level: object = NULL
150+
isolation_level: str(accept={str, NoneType}) = ""
128151
check_same_thread: bool(accept={int}) = True
129152
factory: object(c_default='(PyObject*)clinic_state()->ConnectionType') = ConnectionType
130153
cached_statements: int = 128
@@ -134,10 +157,10 @@ _sqlite3.Connection.__init__ as pysqlite_connection_init
134157
static int
135158
pysqlite_connection_init_impl(pysqlite_Connection *self,
136159
const char *database, double timeout,
137-
int detect_types, PyObject *isolation_level,
160+
int detect_types, const char *isolation_level,
138161
int check_same_thread, PyObject *factory,
139162
int cached_statements, int uri)
140-
/*[clinic end generated code: output=bc39e55eb0b68783 input=f8d1f7efc0d84104]*/
163+
/*[clinic end generated code: output=d8c37afc46d318b0 input=adfb29ac461f9e61]*/
141164
{
142165
int rc;
143166

@@ -148,8 +171,6 @@ pysqlite_connection_init_impl(pysqlite_Connection *self,
148171
pysqlite_state *state = pysqlite_get_state_by_type(Py_TYPE(self));
149172
self->state = state;
150173

151-
self->begin_statement = NULL;
152-
153174
Py_CLEAR(self->statement_cache);
154175
Py_CLEAR(self->cursors);
155176

@@ -174,20 +195,16 @@ pysqlite_connection_init_impl(pysqlite_Connection *self,
174195
return -1;
175196
}
176197

177-
if (!isolation_level) {
178-
isolation_level = PyUnicode_FromString("");
179-
if (!isolation_level) {
198+
if (isolation_level) {
199+
const char *stmt = get_begin_statement(isolation_level);
200+
if (stmt == NULL) {
180201
return -1;
181202
}
182-
} else {
183-
Py_INCREF(isolation_level);
203+
self->begin_statement = stmt;
184204
}
185-
Py_CLEAR(self->isolation_level);
186-
if (pysqlite_connection_set_isolation_level(self, isolation_level, NULL) != 0) {
187-
Py_DECREF(isolation_level);
188-
return -1;
205+
else {
206+
self->begin_statement = NULL;
189207
}
190-
Py_DECREF(isolation_level);
191208

192209
self->statement_cache = new_statement_cache(self, cached_statements);
193210
if (self->statement_cache == NULL) {
@@ -268,7 +285,6 @@ static int
268285
connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg)
269286
{
270287
Py_VISIT(Py_TYPE(self));
271-
Py_VISIT(self->isolation_level);
272288
Py_VISIT(self->statement_cache);
273289
Py_VISIT(self->cursors);
274290
Py_VISIT(self->row_factory);
@@ -292,7 +308,6 @@ clear_callback_context(callback_context *ctx)
292308
static int
293309
connection_clear(pysqlite_Connection *self)
294310
{
295-
Py_CLEAR(self->isolation_level);
296311
Py_CLEAR(self->statement_cache);
297312
Py_CLEAR(self->cursors);
298313
Py_CLEAR(self->row_factory);
@@ -1317,7 +1332,12 @@ static PyObject* pysqlite_connection_get_isolation_level(pysqlite_Connection* se
13171332
if (!pysqlite_check_connection(self)) {
13181333
return NULL;
13191334
}
1320-
return Py_NewRef(self->isolation_level);
1335+
if (self->begin_statement != NULL) {
1336+
const char *stmt = self->begin_statement;
1337+
const char *iso_level = begin_stmt_to_isolation_level(stmt);
1338+
return PyUnicode_FromString(iso_level);
1339+
}
1340+
Py_RETURN_NONE;
13211341
}
13221342

13231343
static PyObject* pysqlite_connection_get_total_changes(pysqlite_Connection* self, void* unused)
@@ -1347,53 +1367,40 @@ pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* iso
13471367
PyErr_SetString(PyExc_AttributeError, "cannot delete attribute");
13481368
return -1;
13491369
}
1350-
if (isolation_level == Py_None) {
1351-
/* We might get called during connection init, so we cannot use
1352-
* pysqlite_connection_commit() here. */
1353-
if (self->db && !sqlite3_get_autocommit(self->db)) {
1354-
int rc;
1355-
Py_BEGIN_ALLOW_THREADS
1356-
rc = sqlite3_exec(self->db, "COMMIT", NULL, NULL, NULL);
1357-
Py_END_ALLOW_THREADS
1358-
if (rc != SQLITE_OK) {
1359-
return _pysqlite_seterror(self->state, self->db);
1360-
}
1361-
}
1362-
1370+
if (Py_IsNone(isolation_level)) {
13631371
self->begin_statement = NULL;
1364-
} else {
1365-
const char * const *candidate;
1366-
PyObject *uppercase_level;
1367-
_Py_IDENTIFIER(upper);
1368-
1369-
if (!PyUnicode_Check(isolation_level)) {
1370-
PyErr_Format(PyExc_TypeError,
1371-
"isolation_level must be a string or None, not %.100s",
1372-
Py_TYPE(isolation_level)->tp_name);
1372+
1373+
// Execute a COMMIT to re-enable autocommit mode
1374+
PyObject *res = pysqlite_connection_commit_impl(self);
1375+
if (res == NULL) {
13731376
return -1;
13741377
}
1375-
1376-
uppercase_level = _PyObject_CallMethodIdOneArg(
1377-
(PyObject *)&PyUnicode_Type, &PyId_upper,
1378-
isolation_level);
1379-
if (!uppercase_level) {
1378+
Py_DECREF(res);
1379+
}
1380+
else if (PyUnicode_Check(isolation_level)) {
1381+
Py_ssize_t len;
1382+
const char *cstr_level = PyUnicode_AsUTF8AndSize(isolation_level, &len);
1383+
if (cstr_level == NULL) {
13801384
return -1;
13811385
}
1382-
for (candidate = begin_statements; *candidate; candidate++) {
1383-
if (_PyUnicode_EqualToASCIIString(uppercase_level, *candidate + 6))
1384-
break;
1386+
if (strlen(cstr_level) != (size_t)len) {
1387+
PyErr_SetString(PyExc_ValueError, "embedded null character");
1388+
return -1;
13851389
}
1386-
Py_DECREF(uppercase_level);
1387-
if (!*candidate) {
1390+
const char *stmt = get_begin_statement(cstr_level);
1391+
if (stmt == NULL) {
13881392
PyErr_SetString(PyExc_ValueError,
1389-
"invalid value for isolation_level");
1393+
"isolation_level string must be '', 'DEFERRED', "
1394+
"'IMMEDIATE', or 'EXCLUSIVE'");
13901395
return -1;
13911396
}
1392-
self->begin_statement = *candidate;
1397+
self->begin_statement = stmt;
1398+
}
1399+
else {
1400+
PyErr_SetString(PyExc_TypeError,
1401+
"isolation_level must be str or None");
1402+
return -1;
13931403
}
1394-
1395-
Py_INCREF(isolation_level);
1396-
Py_XSETREF(self->isolation_level, isolation_level);
13971404
return 0;
13981405
}
13991406

Modules/_sqlite/connection.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ typedef struct
4949
* bitwise combination thereof makes sense */
5050
int detect_types;
5151

52-
/* None for autocommit, otherwise a PyUnicode with the isolation level */
53-
PyObject* isolation_level;
54-
5552
/* NULL for autocommit, otherwise a string with the BEGIN statement */
5653
const char* begin_statement;
5754

0 commit comments

Comments
 (0)