Code

Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#2473 closed defect (fixed)

[patch] 'in' QuerySet operator generates invalid SQL for empty list

Reported by: adurdin@… Owned by: mtredinnick
Component: Database layer (models, ORM) Version:
Severity: normal Keywords:
Cc: mir@…, gary.wilson@…, tom@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If an empty iterable is passed to the 'in' operator in a QuerySet method or Q() call, the SQL generated is invalid: 'WHERE ... IN ()'; this results in a ProgrammingError exception:

For example:

>>> Book.objects.filter(id__in=[])
Traceback (most recent call last):
  File "<console>", line 1, in ?
  File "/usr/lib/python2.4/site-packages/django/db/models/query.py", line 97, in __repr__
    return repr(self._get_data())
  File "/usr/lib/python2.4/site-packages/django/db/models/query.py", line 430, in _get_data
    self._result_cache = list(self.iterator())
  File "/usr/lib/python2.4/site-packages/django/db/models/query.py", line 172, in iterator
    cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
  File "/usr/lib/python2.4/site-packages/django/db/backends/util.py", line 12, in execute
    return self.cursor.execute(sql, params)
  File "/usr/lib/python2.4/site-packages/django/db/backends/mysql/base.py", line 35, in execute
    return self.cursor.execute(sql, params)
  File "/usr/lib/python2.4/site-packages/MySQLdb/cursors.py", line 163, in execute
    self.errorhandler(self, exc, value)
  File "/usr/lib/python2.4/site-packages/MySQLdb/connections.py", line 35, in defaulterrorhandler
    raise errorclass, errorvalue
ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '))' at line 1")

The SQL generated in this case was:

SELECT `test_book`.`id`,`test_book`.`title`,`test_book`.`author_id` FROM `test_book` WHERE (`test_book`.`id` IN ())

The attached patch will return '0' instead of '... IN ()' if list(iterable) is empty; i.e. for SQL like this:

SELECT `test_book`.`id`,`test_book`.`title`,`test_book`.`author_id` FROM `test_book` WHERE (0)

Attachments (4)

in_empty_list.diff (708 bytes) - added by adurdin@… 8 years ago.
tests.diff (1.0 KB) - added by Gary Wilson <gary.wilson@…> 7 years ago.
added some tests
where_false.diff (1.7 KB) - added by Gary Wilson <gary.wilson@…> 7 years ago.
using WHERE (false) when empty, with the tests
where_0=1.diff (2.0 KB) - added by Gary Wilson <gary.wilson@…> 7 years ago.
used where 0=1 instead of where false

Download all attachments as: .zip

Change History (12)

Changed 8 years ago by adurdin@…

comment:1 Changed 8 years ago by mtredinnick

  • Owner changed from adrian to mtredinnick

This stuff is undergoing a rewrite at the moment, but I'll make sure to include something like your patch in the process. Thanks.

comment:2 Changed 8 years ago by mir@…

  • Cc mir@… added

comment:3 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added

comment:4 Changed 7 years ago by Gary Wilson <gary.wilson@…>

The patch doesn't work on postgres. It seems to work, though, with SQL like:

SELECT "auth_user"."id","auth_user"."username" FROM "auth_user" WHERE ("auth_user"."username" IN (0));

which would work with this simple patch:

=== modified file 'django/db/models/query.py'
--- django/db/models/query.py   2006-12-19 04:35:09 +0000
+++ django/db/models/query.py   2006-12-19 05:53:41 +0000
@@ -641,7 +641,7 @@
     except KeyError:
         pass
     if lookup_type == 'in':
-        return '%s%s IN (%s)' % (table_prefix, field_name, ','.join(['%s' for v in value]))
+        return '%s%s IN (%s)' % (table_prefix, field_name, ','.join(['%s' for id in value]) or 0)
     elif lookup_type == 'range':
         return '%s%s BETWEEN %%s AND %%s' % (table_prefix, field_name)
     elif lookup_type in ('year', 'month', 'day'):

Can anyone test if this patch also works on MySQL?

Changed 7 years ago by Gary Wilson <gary.wilson@…>

added some tests

comment:5 Changed 7 years ago by Gary Wilson <gary.wilson@…>

My little patch above is incorrect. Would WHERE (false) possibly work cross backend?

Changed 7 years ago by Gary Wilson <gary.wilson@…>

using WHERE (false) when empty, with the tests

comment:6 Changed 7 years ago by Thomas Steinacher <tom@…>

  • Cc tom@… added

Changed 7 years ago by Gary Wilson <gary.wilson@…>

used where 0=1 instead of where false

comment:7 Changed 7 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [4283]) Fixed #2473 -- Added special case for 'in=[]' (empty set) queries, because 'WHERE attr IN ()' is invalid SQL on many backends. Thanks, Gary Wilson.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.