From f901fd52a5c5716dd23664dd751a661b737df73e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 18 May 2021 09:23:59 +0200 Subject: [PATCH 1/5] bpo-44165: optimise sqlite3 statement preparation by passing string size --- Modules/_sqlite/connection.c | 4 ++-- Modules/_sqlite/cursor.c | 14 ++++++++++---- Modules/_sqlite/statement.c | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 28932726b74257..57daa8f5a2243f 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -428,7 +428,7 @@ pysqlite_connection_commit_impl(pysqlite_Connection *self) if (!sqlite3_get_autocommit(self->db)) { Py_BEGIN_ALLOW_THREADS - rc = sqlite3_prepare_v2(self->db, "COMMIT", -1, &statement, NULL); + rc = sqlite3_prepare_v2(self->db, "COMMIT", 7, &statement, NULL); Py_END_ALLOW_THREADS if (rc != SQLITE_OK) { _pysqlite_seterror(self->db); @@ -478,7 +478,7 @@ pysqlite_connection_rollback_impl(pysqlite_Connection *self) pysqlite_do_all_statements(self, ACTION_RESET, 1); Py_BEGIN_ALLOW_THREADS - rc = sqlite3_prepare_v2(self->db, "ROLLBACK", -1, &statement, NULL); + rc = sqlite3_prepare_v2(self->db, "ROLLBACK", 9, &statement, NULL); Py_END_ALLOW_THREADS if (rc != SQLITE_OK) { _pysqlite_seterror(self->db); diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index b71f780a0b4dfd..3506449d9e1dd7 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -681,6 +681,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) const char* script_cstr; sqlite3_stmt* statement; int rc; + Py_ssize_t script_length; PyObject* result; if (!check_cursor(self)) { @@ -690,7 +691,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) self->reset = 0; if (PyUnicode_Check(script_obj)) { - script_cstr = PyUnicode_AsUTF8(script_obj); + script_cstr = PyUnicode_AsUTF8AndSize(script_obj, &script_length); if (!script_cstr) { return NULL; } @@ -706,13 +707,16 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) } Py_DECREF(result); + Py_ssize_t size_incl_null = script_length + 1; while (1) { + const char *tail; + Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(self->connection->db, script_cstr, - -1, + size_incl_null, &statement, - &script_cstr); + &tail); Py_END_ALLOW_THREADS if (rc != SQLITE_OK) { _pysqlite_seterror(self->connection->db); @@ -740,9 +744,11 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) goto error; } - if (*script_cstr == (char)0) { + if (*tail == (char)0) { break; } + size_incl_null -= (tail - script_cstr); + script_cstr = tail; } error: diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 57026270e1eeb5..f1dbca6de77e5d 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -96,7 +96,7 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(connection->db, sql_cstr, - -1, + sql_cstr_len + 1, &self->st, &tail); Py_END_ALLOW_THREADS From 9febd15380ac41ba724cb33d4d01a509a3a37603 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 19 May 2021 22:18:08 +0200 Subject: [PATCH 2/5] Pass -1 if size >= INT_MAX --- Modules/_sqlite/cursor.c | 9 ++++----- Modules/_sqlite/statement.c | 8 +++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 3506449d9e1dd7..56038f94fdf002 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -681,7 +681,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) const char* script_cstr; sqlite3_stmt* statement; int rc; - Py_ssize_t script_length; + Py_ssize_t sql_len; PyObject* result; if (!check_cursor(self)) { @@ -691,7 +691,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) self->reset = 0; if (PyUnicode_Check(script_obj)) { - script_cstr = PyUnicode_AsUTF8AndSize(script_obj, &script_length); + script_cstr = PyUnicode_AsUTF8AndSize(script_obj, &sql_len); if (!script_cstr) { return NULL; } @@ -707,14 +707,13 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) } Py_DECREF(result); - Py_ssize_t size_incl_null = script_length + 1; while (1) { const char *tail; Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(self->connection->db, script_cstr, - size_incl_null, + (sql_len >= INT_MAX) ? -1 : (int)sql_len + 1, &statement, &tail); Py_END_ALLOW_THREADS @@ -747,7 +746,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) if (*tail == (char)0) { break; } - size_incl_null -= (tail - script_cstr); + sql_len -= (tail - script_cstr); script_cstr = tail; } diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index f1dbca6de77e5d..bd1cdd6e62dfcd 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -93,10 +93,16 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con break; } + if (sql_cstr_len >= INT_MAX) { + sql_cstr_len = -1; + } + else { + sql_cstr_len += 1; + } Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(connection->db, sql_cstr, - sql_cstr_len + 1, + (int)sql_cstr_len, &self->st, &tail); Py_END_ALLOW_THREADS From 081de33f3f63598c0f3b5dabd0eddd40fab29f06 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 00:02:55 +0200 Subject: [PATCH 3/5] Raise OverflowError if SQL string is too long --- Modules/_sqlite/connection.c | 3 +++ Modules/_sqlite/connection.h | 3 +++ Modules/_sqlite/cursor.c | 6 +++++- Modules/_sqlite/statement.c | 12 +++++------- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 57daa8f5a2243f..56926171acd73e 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -172,6 +172,9 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, return -1; } + // Fetch the maximum length of an SQL string or BLOB + self->max_length = sqlite3_limit(self->db, SQLITE_LIMIT_LENGTH, -1); + self->Warning = pysqlite_Warning; self->Error = pysqlite_Error; self->InterfaceError = pysqlite_InterfaceError; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 82f6baf6eef3d7..c37e4c6e5c5c05 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -45,6 +45,9 @@ typedef struct /* the timeout value in seconds for database locks */ double timeout; + /* the maximum length of an SQL string or BLOB */ + int max_length; + /* for internal use in the timeout handler: when did the timeout handler * first get called with count=0? */ double timeout_started; diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 56038f94fdf002..485f89a8d90f0d 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -695,6 +695,10 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) if (!script_cstr) { return NULL; } + if (sql_len >= self->connection->max_length) { + PyErr_SetString(PyExc_OverflowError, "query string is too large"); + return NULL; + } } else { PyErr_SetString(PyExc_ValueError, "script argument must be unicode."); return NULL; @@ -713,7 +717,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(self->connection->db, script_cstr, - (sql_len >= INT_MAX) ? -1 : (int)sql_len + 1, + (int)sql_len + 1, &statement, &tail); Py_END_ALLOW_THREADS diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index bd1cdd6e62dfcd..aaf779ee6becc9 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -66,6 +66,10 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con rc = PYSQLITE_SQL_WRONG_TYPE; return rc; } + if (sql_cstr_len >= connection->max_length) { + PyErr_SetString(PyExc_OverflowError, "query string is too large"); + return PYSQLITE_TOO_MUCH_SQL; + } if (strlen(sql_cstr) != (size_t)sql_cstr_len) { PyErr_SetString(PyExc_ValueError, "the query contains a null character"); return PYSQLITE_SQL_WRONG_TYPE; @@ -93,16 +97,10 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con break; } - if (sql_cstr_len >= INT_MAX) { - sql_cstr_len = -1; - } - else { - sql_cstr_len += 1; - } Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(connection->db, sql_cstr, - (int)sql_cstr_len, + (int)sql_cstr_len + 1, &self->st, &tail); Py_END_ALLOW_THREADS From d981be607da735a80ab5b467dfb8e1efb82dbb5f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 00:09:02 +0200 Subject: [PATCH 4/5] Raise DataError iso. OverflowError to keep current behaviour --- Modules/_sqlite/cursor.c | 2 +- Modules/_sqlite/statement.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index 485f89a8d90f0d..bdd33ee6639546 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -696,7 +696,7 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) return NULL; } if (sql_len >= self->connection->max_length) { - PyErr_SetString(PyExc_OverflowError, "query string is too large"); + PyErr_SetString(pysqlite_DataError, "query string is too large"); return NULL; } } else { diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index aaf779ee6becc9..d1aaebdf5e0a5b 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -67,7 +67,7 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con return rc; } if (sql_cstr_len >= connection->max_length) { - PyErr_SetString(PyExc_OverflowError, "query string is too large"); + PyErr_SetString(pysqlite_DataError, "query string is too large"); return PYSQLITE_TOO_MUCH_SQL; } if (strlen(sql_cstr) != (size_t)sql_cstr_len) { From d0d0618b7713161ab2a88b25075f795ae1ba56e2 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 00:16:49 +0200 Subject: [PATCH 5/5] sqlite3_limit is cheap; use it directly iso. caching it in the connection object --- Modules/_sqlite/connection.c | 3 --- Modules/_sqlite/connection.h | 3 --- Modules/_sqlite/cursor.c | 5 ++++- Modules/_sqlite/statement.c | 4 +++- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 56926171acd73e..57daa8f5a2243f 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -172,9 +172,6 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, return -1; } - // Fetch the maximum length of an SQL string or BLOB - self->max_length = sqlite3_limit(self->db, SQLITE_LIMIT_LENGTH, -1); - self->Warning = pysqlite_Warning; self->Error = pysqlite_Error; self->InterfaceError = pysqlite_InterfaceError; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index c37e4c6e5c5c05..82f6baf6eef3d7 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -45,9 +45,6 @@ typedef struct /* the timeout value in seconds for database locks */ double timeout; - /* the maximum length of an SQL string or BLOB */ - int max_length; - /* for internal use in the timeout handler: when did the timeout handler * first get called with count=0? */ double timeout_started; diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index bdd33ee6639546..30e29b2cd0f733 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -695,7 +695,10 @@ pysqlite_cursor_executescript(pysqlite_Cursor *self, PyObject *script_obj) if (!script_cstr) { return NULL; } - if (sql_len >= self->connection->max_length) { + + int max_length = sqlite3_limit(self->connection->db, + SQLITE_LIMIT_LENGTH, -1); + if (sql_len >= max_length) { PyErr_SetString(pysqlite_DataError, "query string is too large"); return NULL; } diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index d1aaebdf5e0a5b..16db81a4440d9f 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -66,7 +66,9 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con rc = PYSQLITE_SQL_WRONG_TYPE; return rc; } - if (sql_cstr_len >= connection->max_length) { + + int max_length = sqlite3_limit(connection->db, SQLITE_LIMIT_LENGTH, -1); + if (sql_cstr_len >= max_length) { PyErr_SetString(pysqlite_DataError, "query string is too large"); return PYSQLITE_TOO_MUCH_SQL; }