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@gmail.com>

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):

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.`

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 @@ skip_optional:
 exit:
     return return_value;
 }
-/*[clinic end generated code: output=9ac18606b0eaec03 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=d7f142e9a7a80468 input=a9049054013a1b77]*/

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
 

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
+// 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.");
 
-    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]*/
+#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 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;
+            }
         }
     }
-    else {
-        Py_INCREF(isolation_level);
-    }
-    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]