Skip to content

bpo-45512: Simplify manage isolation level #29562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 21 additions & 35 deletions Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ class _sqlite3.Connection "pysqlite_Connection *" "clinic_state()->ConnectionTyp

_Py_IDENTIFIER(cursor);

static const char * const begin_statements[] = {
"BEGIN ",
"BEGIN DEFERRED",
"BEGIN IMMEDIATE",
"BEGIN EXCLUSIVE",
NULL
};

static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self);
static void free_callback_context(callback_context *ctx);
static void set_callback_context(callback_context **ctx_pp,
Expand Down Expand Up @@ -108,25 +100,21 @@ new_statement_cache(pysqlite_Connection *self, pysqlite_state *state,
return res;
}

static inline const char *
begin_stmt_to_isolation_level(const char *begin_stmt)
{
assert(begin_stmt != NULL);

// All begin statements start with "BEGIN "; add strlen("BEGIN ") to get
// the isolation level.
return begin_stmt + 6;
}

static const char *
get_begin_statement(const char *level)
get_isolation_level(const char *level)
{
assert(level != NULL);
for (int i = 0; begin_statements[i] != NULL; i++) {
const char *stmt = begin_statements[i];
const char *candidate = begin_stmt_to_isolation_level(stmt);
static const char *const allowed_levels[] = {
"",
"DEFERRED",
"IMMEDIATE",
"EXCLUSIVE",
NULL
};
for (int i = 0; allowed_levels[i] != NULL; i++) {
const char *candidate = allowed_levels[i];
if (sqlite3_stricmp(level, candidate) == 0) {
return begin_statements[i];
return candidate;
}
}
PyErr_SetString(PyExc_ValueError,
Expand Down Expand Up @@ -202,10 +190,10 @@ pysqlite_connection_init_impl(pysqlite_Connection *self,
}

// Convert isolation level to begin statement.
const char *begin_statement = NULL;
const char *level = NULL;
if (isolation_level != NULL) {
begin_statement = get_begin_statement(isolation_level);
if (begin_statement == NULL) {
level = get_isolation_level(isolation_level);
if (level == NULL) {
return -1;
}
}
Expand All @@ -227,7 +215,7 @@ pysqlite_connection_init_impl(pysqlite_Connection *self,
self->db = db;
self->state = state;
self->detect_types = detect_types;
self->begin_statement = begin_statement;
self->isolation_level = level;
self->check_same_thread = check_same_thread;
self->thread_ident = PyThread_get_thread_ident();
self->statement_cache = statement_cache;
Expand Down Expand Up @@ -1345,10 +1333,8 @@ static PyObject* pysqlite_connection_get_isolation_level(pysqlite_Connection* se
if (!pysqlite_check_connection(self)) {
return NULL;
}
if (self->begin_statement != NULL) {
const char *stmt = self->begin_statement;
const char *iso_level = begin_stmt_to_isolation_level(stmt);
return PyUnicode_FromString(iso_level);
if (self->isolation_level != NULL) {
return PyUnicode_FromString(self->isolation_level);
}
Py_RETURN_NONE;
}
Expand Down Expand Up @@ -1381,7 +1367,7 @@ pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* iso
return -1;
}
if (Py_IsNone(isolation_level)) {
self->begin_statement = NULL;
self->isolation_level = NULL;

// Execute a COMMIT to re-enable autocommit mode
PyObject *res = pysqlite_connection_commit_impl(self);
Expand All @@ -1400,11 +1386,11 @@ pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* iso
PyErr_SetString(PyExc_ValueError, "embedded null character");
return -1;
}
const char *stmt = get_begin_statement(cstr_level);
if (stmt == NULL) {
const char *level = get_isolation_level(cstr_level);
if (level == NULL) {
return -1;
}
self->begin_statement = stmt;
self->isolation_level = level;
}
else {
PyErr_SetString(PyExc_TypeError,
Expand Down
6 changes: 3 additions & 3 deletions Modules/_sqlite/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ typedef struct _callback_context
typedef struct
{
PyObject_HEAD
sqlite3* db;
sqlite3 *db;
pysqlite_state *state;

/* the type detection mode. Only 0, PARSE_DECLTYPES, PARSE_COLNAMES or a
* bitwise combination thereof makes sense */
int detect_types;

/* NULL for autocommit, otherwise a string with the BEGIN statement */
const char* begin_statement;
/* NULL for autocommit, otherwise a string with the isolation level */
const char *isolation_level;

/* 1 if a check should be performed for each API call if the connection is
* used from the same thread it was created in */
Expand Down
12 changes: 9 additions & 3 deletions Modules/_sqlite/cursor.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,18 @@ static int check_cursor(pysqlite_Cursor* cur)
static int
begin_transaction(pysqlite_Connection *self)
{
assert(self->isolation_level != NULL);
int rc;

Py_BEGIN_ALLOW_THREADS
sqlite3_stmt *statement;
rc = sqlite3_prepare_v2(self->db, self->begin_statement, -1, &statement,
NULL);
char begin_stmt[16] = "BEGIN ";
#ifdef Py_DEBUG
size_t len = strlen(self->isolation_level);
assert(len <= 9);
#endif
(void)strcat(begin_stmt, self->isolation_level);
rc = sqlite3_prepare_v2(self->db, begin_stmt, -1, &statement, NULL);
if (rc == SQLITE_OK) {
(void)sqlite3_step(statement);
rc = sqlite3_finalize(statement);
Expand Down Expand Up @@ -555,7 +561,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation

/* We start a transaction implicitly before a DML statement.
SELECT is the only exception. See #9924. */
if (self->connection->begin_statement
if (self->connection->isolation_level
&& self->statement->is_dml
&& sqlite3_get_autocommit(self->connection->db))
{
Expand Down