Opened 18 years ago

Closed 18 years ago

Last modified 16 years ago

#2473 closed defect (fixed)

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

Reported by: adurdin@… Owned by: Malcolm Tredinnick
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: no UI/UX: no

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

Download all attachments as: .zip

Change History (12)

by adurdin@…, 18 years ago

Attachment: in_empty_list.diff added

comment:1 by Malcolm Tredinnick, 18 years ago

Owner: changed from Adrian Holovaty to Malcolm Tredinnick

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 by mir@…, 18 years ago

Cc: mir@… added

comment:3 by Gary Wilson <gary.wilson@…>, 18 years ago

Cc: gary.wilson@… added

comment:4 by Gary Wilson <gary.wilson@…>, 18 years ago

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?

by Gary Wilson <gary.wilson@…>, 18 years ago

Attachment: tests.diff added

added some tests

comment:5 by Gary Wilson <gary.wilson@…>, 18 years ago

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

by Gary Wilson <gary.wilson@…>, 18 years ago

Attachment: where_false.diff added

using WHERE (false) when empty, with the tests

comment:6 by Thomas Steinacher <tom@…>, 18 years ago

Cc: tom@… added

by Gary Wilson <gary.wilson@…>, 18 years ago

Attachment: where_0=1.diff added

used where 0=1 instead of where false

comment:7 by Russell Keith-Magee, 18 years ago

Resolution: fixed
Status: newclosed

(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.

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