Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Doc/library/sqlite3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -337,17 +337,24 @@ Connection Objects
:meth:`~Cursor.executescript` method with the given *sql_script*, and
returns the cursor.

.. method:: create_function(name, num_params, func)
.. method:: create_function(name, num_params, func, *, deterministic=False)

Creates a user-defined function that you can later use from within SQL
statements under the function name *name*. *num_params* is the number of
parameters the function accepts (if *num_params* is -1, the function may
take any number of arguments), and *func* is a Python callable that is
called as the SQL function.
called as the SQL function. If *deterministic* is true, the created function
is marked as `deterministic <https://sqlite.org/deterministic.html>`_, which
allows SQLite to perform additional optimizations. This flag is supported by
SQLite 3.8.3 or higher, ``sqlite3.NotSupportedError`` will be raised if used
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest add a link to the NotSupportedError documentation, then I noticed it wasn't documented. I've opened https://bugs.python.org/issue34061

with older versions.

The function can return any of the types supported by SQLite: bytes, str, int,
float and ``None``.

.. versionchanged:: 3.8
The *deterministic* parameter was added.

Example:

.. literalinclude:: ../includes/sqlite3/md5func.py
Expand Down
23 changes: 23 additions & 0 deletions Lib/sqlite3/test/userfunctions.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
# 3. This notice may not be removed or altered from any source distribution.

import unittest
import unittest.mock
import sqlite3 as sqlite

def func_returntext():
Expand Down Expand Up @@ -275,6 +276,28 @@ def CheckAnyArguments(self):
val = cur.fetchone()[0]
self.assertEqual(val, 2)

def CheckFuncDeterministic(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test can be split into three tests:

  1. deterministic=False
  2. deterministic=True if SQLite is older than 3.8.3
  3. deterministic=True if SQLite supports the feature

mock = unittest.mock.Mock()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a mock is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used to test how much times function is called. If function is non-deterministic, query can't be optimized, so function have to be called 2 times (and actually there is no need to call it at all for this query :-).
This test relies on SQLite's behavior in some degree, but I did't find out a better way to test it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clarification.

Is it worth to add a test for deterministic=False (or for not specified)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can do something like the following instead of using mock:

def foo():
    foo.calls.append(True)
foo.calls = []

Then we can check the call count as:

self.assertEqual(len(foo.calls), 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serhiy-storchaka added test for it.
@berkerpeksag not sure why this could be better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@berkerpeksag This is how I would write this test. But I just don't use mocks much. I think using a mock here is good.

mock.return_value = None
query = "select deterministic() = deterministic()"

self.con.create_function("deterministic", 0, mock, deterministic=False)
self.con.execute(query)
self.assertEqual(mock.call_count, 2)

if sqlite.sqlite_version_info < (3, 8, 3):
with self.assertRaises(sqlite.NotSupportedError):
self.con.create_function("deterministic", 0, mock, deterministic=True)
else:
self.con.create_function("deterministic", 0, mock, deterministic=True)
mock.reset_mock()
self.con.execute(query)
self.assertEqual(mock.call_count, 1)

def CheckFuncDeterministicKwOnly(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Kw -> Keyword

with self.assertRaises(TypeError):
self.con.create_function("deterministic", 0, int, True)


class AggregateTests(unittest.TestCase):
def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add the parameter *deterministic* to the
:meth:`sqlite3.Connection.create_function` method. Patch by Sergey Fedoseev.
32 changes: 28 additions & 4 deletions Modules/_sqlite/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -810,24 +810,48 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self)

PyObject* pysqlite_connection_create_function(pysqlite_Connection* self, PyObject* args, PyObject* kwargs)
{
static char *kwlist[] = {"name", "narg", "func", NULL, NULL};
static char *kwlist[] = {"name", "narg", "func", "deterministic", NULL};

PyObject* func;
char* name;
int narg;
int rc;
int deterministic = 0;
int flags = SQLITE_UTF8;

if (!pysqlite_check_thread(self) || !pysqlite_check_connection(self)) {
return NULL;
}

if (!PyArg_ParseTupleAndKeywords(args, kwargs, "siO", kwlist,
&name, &narg, &func))
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "siO|$p", kwlist,
&name, &narg, &func, &deterministic))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be safer to use a keyword only parameter, to prevent bugs?

{
return NULL;
}

rc = sqlite3_create_function(self->db, name, narg, SQLITE_UTF8, (void*)func, _pysqlite_func_callback, NULL, NULL);
if (deterministic) {
#if SQLITE_VERSION_NUMBER < 3008003
PyErr_SetString(pysqlite_NotSupportedError,
"deterministic=True requires SQLite 3.8.3 or higher");
return NULL;
#else
if (sqlite3_libversion_number() < 3008003) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually don't check for runtime library version when doing feature detection, but I think this is better. (Note that the only other check was added only in 2016: 7bea234)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for the runtime check

Checking the runtime version is fine. SQLITE_VERSION_NUMBER is the build version. Using Linux packages, it's common that the machine building packages and the users installing packages use different versions.

It also seems risky to use the flag on old versions according to the discussion in this PR.

PyErr_SetString(pysqlite_NotSupportedError,
"deterministic=True requires SQLite 3.8.3 or higher");
return NULL;
}
flags |= SQLITE_DETERMINISTIC;
#endif
}

rc = sqlite3_create_function(self->db,
name,
narg,
flags,
(void*)func,
_pysqlite_func_callback,
NULL,
NULL);

if (rc != SQLITE_OK) {
/* Workaround for SQLite bug: no error code or string is available here */
Expand Down