Opened 6 years ago

Closed 6 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 by Carlton Gibson, 6 years ago

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.

Version 0, edited 6 years ago by Carlton Gibson (next)

comment:2 by Carlton Gibson, 6 years ago

Cc: Carlton Gibson added

in reply to:  1 comment:3 by Sergey Fedoseev, 6 years ago

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 6 years ago by Sergey Fedoseev (previous) (diff)

comment:4 by Carlton Gibson, 6 years ago

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 by Srinivas Reddy Thatiparthy, 6 years ago

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

comment:6 by Nick Pope, 6 years ago

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 by Srinivas Reddy Thatiparthy, 6 years ago

Owner: Srinivas Reddy Thatiparthy removed
Status: assignednew

comment:10 by Nick Pope, 6 years ago

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

comment:11 by Tim Graham, 6 years ago

Patch needs improvement: set

comment:12 by Nick Pope, 6 years ago

Patch needs improvement: unset

comment:13 by Tim Graham <timograham@…>, 6 years ago

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