#29500 closed Bug (fixed)

SQLite functions crashes on NULL values

Reported by: Sergey Fedoseev Owned by: Nick Pope
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords:
Cc: Carlton Gibson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Change History (11)

comment:1 Changed 17 months ago by Carlton Gibson

Hmmm. Not sure we'll be able to do anything about this. (Postgres certainly behaves better.)

Could you 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.

Last edited 17 months ago by Carlton Gibson (previous) (diff)

comment:2 Changed 17 months ago by Carlton Gibson

Cc: Carlton Gibson added

comment:3 in reply to:  1 Changed 17 months ago by Sergey Fedoseev

Replying to Carlton Gibson:

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'

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:

Last edited 17 months ago by Sergey Fedoseev (previous) (diff)

comment:4 Changed 17 months ago by Carlton Gibson

Triage Stage: UnreviewedAccepted

OK, thanks for the clarification. We could certainly consider a PR adding test cases and a bulletproofing to the functions we're shipping.

comment:5 Changed 17 months ago by Srinivas Reddy Thatiparthy

Owner: changed from nobody to Srinivas Reddy Thatiparthy
Status: newassigned

comment:6 Changed 17 months ago 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 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 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 PR/9622 which I am reviewing, particular with respect to my comment.

comment:7 Changed 15 months ago by Srinivas Reddy Thatiparthy

Owner: Srinivas Reddy Thatiparthy deleted
Status: assignednew

comment:10 Changed 15 months ago by Nick Pope

Has patch: set
Owner: set to Nick Pope
Status: newassigned

comment:11 Changed 15 months ago by Tim Graham

Patch needs improvement: set

comment:12 Changed 15 months ago by Nick Pope

Patch needs improvement: unset

comment:13 Changed 14 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 34d6bce:

Fixed #29500 -- Fixed SQLite function crashes on null values.

Co-authored-by: Srinivas Reddy Thatiparthy <thatiparthysreenivas@…>
Co-authored-by: Nick Pope <nick.pope@…>

Note: See TracTickets for help on using tickets.
Back to Top