This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Exception from sqlite3 adapter masked by sqlite3.InterfaceError
Type: behavior Stage: resolved
Components: Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: inglesp, mdk, palaviv
Priority: normal Keywords: patch

Created on 2016-11-17 23:25 by inglesp, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue28729.diff mdk, 2016-11-18 22:40 review
Messages (5)
msg281066 - (view) Author: Peter Inglesby (inglesp) * Date: 2016-11-17 23:25
The following code raises `sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.` when I would expect it to raise `AssertionError: Problem in adapter`.


import sqlite3

class Point:
    def __init__(self, x, y):
        self.x, self.y = x, y

def adapt_point(point):
    assert False, 'Problem in adapter'

sqlite3.register_adapter(Point, adapt_point)

con = sqlite3.connect(":memory:")
cur = con.cursor()

p = Point(4.0, -3.2)
cur.execute("select ?", (p,))
msg281068 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2016-11-18 00:02
Problems looks from `Modules/_sqlite/statement.c`:

```
if (!_need_adapt(current_param)) {
    adapted = current_param;
} else {
    adapted = pysqlite_microprotocols_adapt(current_param, (PyObject*)&pysqlite_PrepareProtocolType, NULL);
    if (adapted) {
        Py_DECREF(current_param);
    } else {
        PyErr_Clear();
        adapted = current_param;
    }
}
```

It has not changed since 2006, since e200ab5b3e (backport from pysqlite2 SVN repository).

I read it as "If an parameter needs an adapter, and the adapter fails, that's OK, continue with the original (unadapted) parameter.", which looks wrong to me, but I may miss something obvious? I tried to set an error instead of restoring the `current_param` and the 261 tests went well, but I'm not yet aware of the coverage of adapters in tests.
msg281069 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2016-11-18 00:10
I missed an occurrence of this "if/else" block, and by changing it, a lot of tests are failing, typically:

```======================================================================
ERROR: CheckBlob (sqlite3.test.types.SqliteTypeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mdk/cpython/Lib/sqlite3/test/types.py", line 72, in CheckBlob
    self.cur.execute("insert into test(b) values (?)", (val,))
sqlite3.InterfaceError: Problem in adapter

```

So this "if/else" giving back the unadapted parameter even when need_adapt is true is necessary, we'll have to understand why and how we can properly detect an adapter failure.
msg281176 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2016-11-18 22:40
By moving:

```
/* else set the right exception and return NULL */
PyErr_SetString(pysqlite_ProgrammingError, "can't adapt");
```

from `pysqlite_microprotocols_adapt` to `pysqlite_adapt` (to avoid changing the semantics from the outside) make the `pysqlite_microprotocols_adapt`protocol clearer:
 - if it went well return something
 - If it went well but had nothing to do: return NULL
 - If it broke, set an exception and return NULL

It does not break any test. Then in `Modules/_sqlite/statement.c`, we can stop blindly muting exceptions with the `PyErr_Clear`s, replacing them with ``if (PyErr_Occurred()) return;``. Again it does not break any test.

I added a test to check that if an adapter raises an exception it bubbles outside the execute method, and it passes.

Sample code given by the issue now gives::

   $ ./python issue28729.py 
   Traceback (most recent call last):
     File "issue28729.py", line 18, in <module>
       cur.execute("select ?", (p,))
     File "issue28729.py", line 10, in adapt_point
       assert False, 'Problem in adapter'
   AssertionError: Problem in adapter

I did not found precise documentation about pysqlite_microprotocols_adapt or pysqlite_adapt, so my changes may break a "well known protocol", but it looks safe as I do not change the _sqlite protocol as far as I can tell, (only when the exception comes directly from the adaptor, obviously), and there were no other uses of pysqlite_microprotocols_adapt, which is not exposed in the module.
msg341568 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2019-05-06 17:26
This has been fixed in fc662ac332443a316a120fa5287c235dc4f8739b.
History
Date User Action Args
2022-04-11 14:58:39adminsetgithub: 72915
2019-05-06 17:26:10mdksetstatus: open -> closed
resolution: fixed
messages: + msg341568

stage: resolved
2016-11-19 15:15:37palavivsetnosy: + palaviv
2016-11-18 22:40:09mdksetfiles: + issue28729.diff
keywords: + patch
messages: + msg281176
2016-11-18 00:10:56mdksetmessages: + msg281069
2016-11-18 00:02:49mdksetnosy: + mdk
messages: + msg281068
2016-11-17 23:25:40inglespcreate