[Django] #29500: SQLite functions crashes on NULL values
Django
-------------------------------------+-------------------------------------
Reporter: Sergey | Owner: nobody
Fedoseev |
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
{{{
In [14]: TestModel2.objects.annotate(null=models.Value(None,
output_field=models.IntegerField())).values(pow=models.F('null') **
models.F('null')).first()
---------------------------------------------------------------------------
OperationalError Traceback (most recent call
last)
~/dev/django/django/db/backends/utils.py in _execute(self, sql, params,
*ignored_wrapper_args)
84 else:
---> 85 return self.cursor.execute(sql, params)
86
~/dev/django/django/db/backends/sqlite3/base.py in execute(self, query,
params)
295 query = self.convert_query(query)
--> 296 return Database.Cursor.execute(self, query, params)
297
OperationalError: user-defined function raised exception
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29500>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hmmm. Not sure we'll be able to do anything about this. (Postgres
certainly behaves better.)
Could you
[https://docs.python.org/3/library/sqlite3.html#sqlite3.enable_callback_tracebacks
enable callback trackbacks] on the client? We can then see the error.
I'll guess it'll be this:
```
>>> None ** None
Traceback (most recent call last):
File "<console>", line 1, in <module>
TypeError: unsupported operand type(s) for ** or pow(): 'NoneType' and
'NoneType'
```
If so we may just have to workaround it by using a function for `pow`
which checks for `None`.
This works:
```
>>>
TestModel.objects.annotate(null=Value(None,output_field=models.IntegerField())).values(pow=models.F('null')).first()
{'pow': None}
```
So it's just the `**` operation.
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sergey Fedoseev):
Replying to [comment:1 Carlton Gibson]:
> I'll guess it'll be this:
It is.
By `SQLite functions` I meant user-defined function created here:
https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L158-L175
The list (incomplete?) of functions that crash on NULL values:
*
[https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L449
_sqlite_time_diff]
*
[https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L464
_sqlite_timestamp_diff]
*
[https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L474
_sqlite_lpad]
*
[https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L480
_sqlite_rpad]
*
[https://github.com/django/django/blob/6dd4edb1b4f5441c5f543e29395039839c50d10b/django/db/backends/sqlite3/base.py#L484
_sqlite_power]
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
OK, thanks for the clarification. We could certainly consider a PR adding
test cases and a bulletproofing to the functions we're shipping.
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:4>
Django
-------------------------------------+-------------------------------------
| Reddy Thatiparthy
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Srinivas Reddy Thatiparthy
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:5>
Django
-------------------------------------+-------------------------------------
| Reddy Thatiparthy
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nick Pope):
We need to be careful how we handle this to ensure that the behaviour
mirrors other backends.
I've checked PostgreSQL and when any one of the arguments to `POWER()`.
`LPAD()` or `RPAD()` is `NULL` they return `NULL`.
We should ensure that we check whether any one of the arguments is `None`
and, if so, return `None`.
We must not catch exceptions such as `TypeError` or `ValueError` to do
this as has been done in the initial version of the
[https://github.com/django/django/pull/10121 PR].
If we were to pass a string to `_sqlite_power()` we would expect a
`TypeError` which should blow up, not return `None`. Compare to
PostgreSQL:
{{{
postgres=# select power(2, 'abc');
ERROR: invalid input syntax for type double precision: "abc"
LINE 1: select power(2, 'abc');
^
}}}
The second part of the problem here is that the sqlite backend suppresses
the error message and returns a different exception:
{{{
OperationalError: user-defined function raised exception
}}}
Obviously this is not particularly helpful, but a quick search and I found
the following on Stack Overflow: https://stackoverflow.com/a/45834923
It points to the documentation for
[https://docs.python.org/3/library/sqlite3.html#sqlite3.enable_callback_tracebacks
sqlite3.enable_callback_tracebacks()].
I would recommend the following:
1. Creation of a decorator to check for `None` passed into any of the
arguments which returns `None` or calls the function as appropriate.
2. Enabling callbacks on tracebacks for sqlite3 (always / when debug
enabled / documentation change to give instruction).
Note that the outcome of this pull request will affect
[https://github.com/django/django/pull/9622 PR/9622] which I am reviewing,
particular with respect to my
[https://github.com/django/django/pull/9622#discussion_r165360894
comment].
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:6>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Srinivas Reddy Thatiparthy => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Jeff
* status: new => assigned
Comment:
I'll be helping Srinivas get this across the finish line.
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Jeff => (none)
* status: assigned => new
Comment:
Sensing a pattern. Deassigning.
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:9>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Nick Pope
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10301 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:10>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:12>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"34d6bceec46c5d4234c156ed682573d2e5de474a" 34d6bce]:
{{{
#!CommitTicketReference repository=""
revision="34d6bceec46c5d4234c156ed682573d2e5de474a"
Fixed #29500 -- Fixed SQLite function crashes on null values.
Co-authored-by: Srinivas Reddy Thatiparthy
<thatiparth...@gmail.com>
Co-authored-by: Nick Pope <nick...@flightdataservices.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29500#comment:13>