From 8bc3b12c36cc4eb44f3679149ce8a962102425f7 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 15 Jul 2021 01:43:51 +0200 Subject: [PATCH 1/2] Migrate to sqlite3_create_collation_v2 This implies that SQLite now takes care of destroying the callback context (the PyObject callable it has been passed), so we can strip the collation dict from the connection object. --- Modules/_sqlite/connection.c | 41 +++++++++++++++--------------------- Modules/_sqlite/connection.h | 3 --- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 64bd53aea7ea6a..555d5d5cec2510 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -177,11 +177,6 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->function_pinboard_progress_handler = NULL; self->function_pinboard_authorizer_cb = NULL; - Py_XSETREF(self->collations, PyDict_New()); - if (!self->collations) { - return -1; - } - pysqlite_state *state = pysqlite_get_state(NULL); self->Warning = state->Warning; self->Error = state->Error; @@ -249,7 +244,6 @@ connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) Py_VISIT(self->function_pinboard_trace_callback); Py_VISIT(self->function_pinboard_progress_handler); Py_VISIT(self->function_pinboard_authorizer_cb); - Py_VISIT(self->collations); return 0; } @@ -265,7 +259,6 @@ connection_clear(pysqlite_Connection *self) Py_CLEAR(self->function_pinboard_trace_callback); Py_CLEAR(self->function_pinboard_progress_handler); Py_CLEAR(self->function_pinboard_authorizer_cb); - Py_CLEAR(self->collations); return 0; } @@ -1761,29 +1754,29 @@ pysqlite_connection_create_collation_impl(pysqlite_Connection *self, if (!uppercase_name_str) goto finally; - if (callable != Py_None && !PyCallable_Check(callable)) { - PyErr_SetString(PyExc_TypeError, "parameter must be callable"); - goto finally; + int flags = SQLITE_UTF8; + if (callable == Py_None) { + rc = sqlite3_create_collation_v2(self->db, uppercase_name_str, flags, + NULL, NULL, NULL); } - - if (callable != Py_None) { - if (PyDict_SetItem(self->collations, uppercase_name, callable) == -1) - goto finally; - } else { - if (PyDict_DelItem(self->collations, uppercase_name) == -1) + else { + if (!PyCallable_Check(callable)) { + PyErr_SetString(PyExc_TypeError, "parameter must be callable"); goto finally; + } + rc = sqlite3_create_collation_v2(self->db, uppercase_name_str, flags, + Py_NewRef(callable), + &pysqlite_collation_callback, + &_destructor); } - rc = sqlite3_create_collation(self->db, - uppercase_name_str, - SQLITE_UTF8, - (callable != Py_None) ? callable : NULL, - (callable != Py_None) ? pysqlite_collation_callback : NULL); if (rc != SQLITE_OK) { + /* Unlike other sqlite3_* functions, the destructor callback is _not_ + * called if sqlite3_create_collation_v2() fails, so we have to free + * the context before returning. + */ if (callable != Py_None) { - if (PyDict_DelItem(self->collations, uppercase_name) < 0) { - PyErr_Clear(); - } + _destructor(callable); } _pysqlite_seterror(self->db); goto finally; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 5e7e7e5abe0668..50c2015be3eb19 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -82,9 +82,6 @@ typedef struct PyObject* function_pinboard_progress_handler; PyObject* function_pinboard_authorizer_cb; - /* a dictionary of registered collation name => collation callable mappings */ - PyObject* collations; - /* Exception objects: borrowed refs. */ PyObject* Warning; PyObject* Error; From 9af994030f9d0d9b03b77f053c3e3dcc7c340e38 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 20 Jul 2021 22:31:26 +0200 Subject: [PATCH 2/2] Address review: Use Py_DECREF iso. _destructor() --- Modules/_sqlite/connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 555d5d5cec2510..f05db89b86d152 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1776,7 +1776,7 @@ pysqlite_connection_create_collation_impl(pysqlite_Connection *self, * the context before returning. */ if (callable != Py_None) { - _destructor(callable); + Py_DECREF(callable); } _pysqlite_seterror(self->db); goto finally;