Skip to content

Commit

Permalink
gh-95132: Correctly relay *args and **kwds from sqlite3.connect to fa…
Browse files Browse the repository at this point in the history
…ctory (#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 <[email protected]>
  • Loading branch information
Erlend Egeberg Aasland and serhiy-storchaka authored Jul 23, 2022
1 parent c1e9298 commit a3d4d15
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 147 deletions.
20 changes: 20 additions & 0 deletions Lib/test/test_sqlite3/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.`
112 changes: 1 addition & 111 deletions Modules/_sqlite/clinic/module.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 33 additions & 36 deletions Modules/_sqlite/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit a3d4d15

Please sign in to comment.