Opened 5 years ago

Closed 5 years ago

#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 5 years 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 5 years ago by Carlton Gibson (previous) (diff)

comment:2 Changed 5 years ago by Carlton Gibson

Cc: Carlton Gibson added

comment:3 in reply to:  1 Changed 5 years ago by Sergey Fedoseev

Replying to 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:

Version 0, edited 5 years ago by Sergey Fedoseev (next)

comment:4 Changed 5 years 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 5 years ago by Srinivas Reddy Thatiparthy

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

comment:6 Changed 5 years 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 5 years ago by Srinivas Reddy Thatiparthy

Owner: Srinivas Reddy Thatiparthy deleted
Status: assignednew

comment:10 Changed 5 years ago by Nick Pope

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

comment:11 Changed 5 years ago by Tim Graham

Patch needs improvement: set

comment:12 Changed 5 years ago by Nick Pope

Patch needs improvement: unset

comment:13 Changed 5 years 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