[Python-checkins] gh-95132: Correctly relay *args and **kwds from sqlite3.connect to factory (#95146)
erlend-aasland
webhook-mailer at python.org
Sat Jul 23 03:51:33 EDT 2022
https://github.com/python/cpython/commit/a3d4d15f53777662ce0957500e5a538ce7161f5e
commit: a3d4d15f53777662ce0957500e5a538ce7161f5e
branch: main
author: Erlend Egeberg Aasland <erlend.aasland at innova.no>
committer: erlend-aasland <erlend.aasland at protonmail.com>
date: 2022-07-23T09:51:28+02:00
summary:
gh-95132: Correctly relay *args and **kwds from sqlite3.connect to factory (#95146)
This PR partially reverts gh-24421 (PR) and fixes the remaining concerns
given in gh-93044 (issue):
- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
is passed to factory()
Co-authored-by: Serhiy Storchaka <storchaka at gmail.com>
files:
A Misc/NEWS.d/next/Library/2022-07-22-21-18-17.gh-issue-95132.n9anlw.rst
M Lib/test/test_sqlite3/test_factory.py
M Modules/_sqlite/clinic/module.c.h
M Modules/_sqlite/connection.c
M Modules/_sqlite/module.c
diff --git a/Lib/test/test_sqlite3/test_factory.py b/Lib/test/test_sqlite3/test_factory.py
index 71603faa02840..7fdc45ab69243 100644
--- a/Lib/test/test_sqlite3/test_factory.py
+++ b/Lib/test/test_sqlite3/test_factory.py
@@ -50,6 +50,26 @@ def __init__(self, *args, **kwargs):
con = sqlite.connect(":memory:", factory=factory)
self.assertIsInstance(con, factory)
+ def test_connection_factory_relayed_call(self):
+ # gh-95132: keyword args must not be passed as positional args
+ class Factory(sqlite.Connection):
+ def __init__(self, *args, **kwargs):
+ kwargs["isolation_level"] = None
+ super(Factory, self).__init__(*args, **kwargs)
+
+ con = sqlite.connect(":memory:", factory=Factory)
+ self.assertIsNone(con.isolation_level)
+ self.assertIsInstance(con, Factory)
+
+ def test_connection_factory_as_positional_arg(self):
+ class Factory(sqlite.Connection):
+ def __init__(self, *args, **kwargs):
+ super(Factory, self).__init__(*args, **kwargs)
+
+ con = sqlite.connect(":memory:", 5.0, 0, None, True, Factory)
+ self.assertIsNone(con.isolation_level)
+ self.assertIsInstance(con, Factory)
+
class CursorFactoryTests(unittest.TestCase):
def setUp(self):
diff --git a/Misc/NEWS.d/next/Library/2022-07-22-21-18-17.gh-issue-95132.n9anlw.rst b/Misc/NEWS.d/next/Library/2022-07-22-21-18-17.gh-issue-95132.n9anlw.rst
new file mode 100644
index 0000000000000..64666ad84fb4a
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-07-22-21-18-17.gh-issue-95132.n9anlw.rst
@@ -0,0 +1,4 @@
+Fix a :mod:`sqlite3` regression where ``*args`` and ``**kwds`` were
+incorrectly relayed from :py:func:`~sqlite3.connect` to the
+:class:`~sqlite3.Connection` factory. The regression was introduced in 3.11a1
+with PR 24421 (:gh:`85128`). Patch by Erlend E. Aasland.`
diff --git a/Modules/_sqlite/clinic/module.c.h b/Modules/_sqlite/clinic/module.c.h
index ef0dd4d1c103e..3e932a6117eb2 100644
--- a/Modules/_sqlite/clinic/module.c.h
+++ b/Modules/_sqlite/clinic/module.c.h
@@ -2,116 +2,6 @@
preserve
[clinic start generated code]*/
-PyDoc_STRVAR(pysqlite_connect__doc__,
-"connect($module, /, database, timeout=5.0, detect_types=0,\n"
-" isolation_level=<unrepresentable>, check_same_thread=True,\n"
-" factory=ConnectionType, cached_statements=128, uri=False)\n"
-"--\n"
-"\n"
-"Opens a connection to the SQLite database file database.\n"
-"\n"
-"You can use \":memory:\" to open a database connection to a database that resides\n"
-"in RAM instead of on disk.");
-
-#define PYSQLITE_CONNECT_METHODDEF \
- {"connect", _PyCFunction_CAST(pysqlite_connect), METH_FASTCALL|METH_KEYWORDS, pysqlite_connect__doc__},
-
-static PyObject *
-pysqlite_connect_impl(PyObject *module, PyObject *database, double timeout,
- int detect_types, PyObject *isolation_level,
- int check_same_thread, PyObject *factory,
- int cached_statements, int uri);
-
-static PyObject *
-pysqlite_connect(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
-{
- PyObject *return_value = NULL;
- static const char * const _keywords[] = {"database", "timeout", "detect_types", "isolation_level", "check_same_thread", "factory", "cached_statements", "uri", NULL};
- static _PyArg_Parser _parser = {NULL, _keywords, "connect", 0};
- PyObject *argsbuf[8];
- Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1;
- PyObject *database;
- double timeout = 5.0;
- int detect_types = 0;
- PyObject *isolation_level = NULL;
- int check_same_thread = 1;
- PyObject *factory = (PyObject*)clinic_state()->ConnectionType;
- int cached_statements = 128;
- int uri = 0;
-
- args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 8, 0, argsbuf);
- if (!args) {
- goto exit;
- }
- database = args[0];
- if (!noptargs) {
- goto skip_optional_pos;
- }
- if (args[1]) {
- if (PyFloat_CheckExact(args[1])) {
- timeout = PyFloat_AS_DOUBLE(args[1]);
- }
- else
- {
- timeout = PyFloat_AsDouble(args[1]);
- if (timeout == -1.0 && PyErr_Occurred()) {
- goto exit;
- }
- }
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- if (args[2]) {
- detect_types = _PyLong_AsInt(args[2]);
- if (detect_types == -1 && PyErr_Occurred()) {
- goto exit;
- }
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- if (args[3]) {
- isolation_level = args[3];
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- if (args[4]) {
- check_same_thread = _PyLong_AsInt(args[4]);
- if (check_same_thread == -1 && PyErr_Occurred()) {
- goto exit;
- }
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- if (args[5]) {
- factory = args[5];
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- if (args[6]) {
- cached_statements = _PyLong_AsInt(args[6]);
- if (cached_statements == -1 && PyErr_Occurred()) {
- goto exit;
- }
- if (!--noptargs) {
- goto skip_optional_pos;
- }
- }
- uri = PyObject_IsTrue(args[7]);
- if (uri < 0) {
- goto exit;
- }
-skip_optional_pos:
- return_value = pysqlite_connect_impl(module, database, timeout, detect_types, isolation_level, check_same_thread, factory, cached_statements, uri);
-
-exit:
- return return_value;
-}
-
PyDoc_STRVAR(pysqlite_complete_statement__doc__,
"complete_statement($module, /, statement)\n"
"--\n"
@@ -292,4 +182,4 @@ pysqlite_adapt(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
exit:
return return_value;
}
-/*[clinic end generated code: output=9ac18606b0eaec03 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=d7f142e9a7a80468 input=a9049054013a1b77]*/
diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
index b6b7417fc0a6c..ceb77bbf8420f 100644
--- a/Modules/_sqlite/connection.c
+++ b/Modules/_sqlite/connection.c
@@ -140,6 +140,7 @@ class IsolationLevel_converter(CConverter):
[python start generated code]*/
/*[python end generated code: output=da39a3ee5e6b4b0d input=cbcfe85b253061c2]*/
+// NB: This needs to be in sync with the sqlite3.connect docstring
/*[clinic input]
_sqlite3.Connection.__init__ as pysqlite_connection_init
diff --git a/Modules/_sqlite/module.c b/Modules/_sqlite/module.c
index eca25b94a4817..707aae160ab4f 100644
--- a/Modules/_sqlite/module.c
+++ b/Modules/_sqlite/module.c
@@ -42,47 +42,44 @@ module _sqlite3
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=81e330492d57488e]*/
-// NOTE: This must equal sqlite3.Connection.__init__ argument spec!
-/*[clinic input]
-_sqlite3.connect as pysqlite_connect
-
- database: object
- timeout: double = 5.0
- detect_types: int = 0
- isolation_level: object = NULL
- check_same_thread: bool(accept={int}) = True
- factory: object(c_default='(PyObject*)clinic_state()->ConnectionType') = ConnectionType
- cached_statements: int = 128
- uri: bool = False
-
-Opens a connection to the SQLite database file database.
-
-You can use ":memory:" to open a database connection to a database that resides
-in RAM instead of on disk.
-[clinic start generated code]*/
+// NB: This needs to be in sync with the Connection.__init__ docstring.
+PyDoc_STRVAR(module_connect_doc,
+"connect($module, /, database, timeout=5.0, detect_types=0,\n"
+" isolation_level='', check_same_thread=True,\n"
+" factory=ConnectionType, cached_statements=128, uri=False)\n"
+"--\n"
+"\n"
+"Opens a connection to the SQLite database file database.\n"
+"\n"
+"You can use \":memory:\" to open a database connection to a database that resides\n"
+"in RAM instead of on disk.");
+
+#define PYSQLITE_CONNECT_METHODDEF \
+ {"connect", _PyCFunction_CAST(module_connect), METH_FASTCALL|METH_KEYWORDS, module_connect_doc},
static PyObject *
-pysqlite_connect_impl(PyObject *module, PyObject *database, double timeout,
- int detect_types, PyObject *isolation_level,
- int check_same_thread, PyObject *factory,
- int cached_statements, int uri)
-/*[clinic end generated code: output=450ac9078b4868bb input=e16914663ddf93ce]*/
+module_connect(PyObject *module, PyObject *const *args, Py_ssize_t nargsf,
+ PyObject *kwnames)
{
- if (isolation_level == NULL) {
- isolation_level = PyUnicode_FromString("");
- if (isolation_level == NULL) {
- return NULL;
- }
+ pysqlite_state *state = pysqlite_get_state(module);
+ PyObject *factory = (PyObject *)state->ConnectionType;
+
+ static const int FACTORY_POS = 5;
+ Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
+ if (nargs > FACTORY_POS) {
+ factory = args[FACTORY_POS];
}
- else {
- Py_INCREF(isolation_level);
+ else if (kwnames != NULL) {
+ for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(kwnames); i++) {
+ PyObject *item = PyTuple_GET_ITEM(kwnames, i); // borrowed ref.
+ if (PyUnicode_CompareWithASCIIString(item, "factory") == 0) {
+ factory = args[nargs + i];
+ break;
+ }
+ }
}
- PyObject *res = PyObject_CallFunction(factory, "OdiOiOii", database,
- timeout, detect_types,
- isolation_level, check_same_thread,
- factory, cached_statements, uri);
- Py_DECREF(isolation_level);
- return res;
+
+ return PyObject_Vectorcall(factory, args, nargsf, kwnames);
}
/*[clinic input]
More information about the Python-checkins
mailing list