diff --git a/django/db/models/query.py b/django/db/models/query.py index cafec42697..ffea11eeb7 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -88,7 +88,6 @@ class _QuerySet(object): max_depth = self.query.max_depth index_end = len(self.model._meta.fields) extra_select = self.query.extra_select.keys() - extra_select.sort() for row in self.query.results_iter(): if fill_cache: obj, index_end = get_cached_row(klass=self.model, row=row, @@ -378,11 +377,7 @@ class ValuesQuerySet(QuerySet): # names of the model fields to select. def iterator(self): - extra_select = self.query.extra_select.keys() - extra_select.sort() - if extra_select: - self.field_names.extend([f for f in extra_select]) - + self.field_names.extend([f for f in self.query.extra_select.keys()]) for row in self.query.results_iter(): yield dict(zip(self.field_names, row)) @@ -409,7 +404,7 @@ class ValuesQuerySet(QuerySet): except KeyError, e: raise FieldDoesNotExist('%s has no field named %r' % (opts.object_name, e.args[0])) - field_names = self._fields + field_names = list(self._fields) else: fields = [] field_names = [] diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index c633b8849f..412fe50e21 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -10,7 +10,8 @@ all about the internals of models in order to get the information it needs. import copy import re -from django.utils import tree +from django.utils.tree import Node +from django.utils.datastructures import SortedDict from django.db.models.sql.where import WhereNode, AND, OR from django.db.models.sql.datastructures import Count, Date from django.db.models.fields import FieldDoesNotExist, Field @@ -94,7 +95,7 @@ class Query(object): # These are for extensions. The contents are more or less appended # verbatim to the appropriate clause. - self.extra_select = {} # Maps col_alias -> col_sql. + self.extra_select = SortedDict() # Maps col_alias -> col_sql. self.extra_tables = [] self.extra_where = [] self.extra_params = [] @@ -364,12 +365,8 @@ class Query(object): for f in self.model._meta.fields] aliases = result[:] - # We sort extra_select so that the result columns are in a well-defined - # order (and thus QuerySet.iterator can extract them correctly). - extra_select = self.extra_select.items() - extra_select.sort() result.extend(['(%s) AS %s' % (col, alias) - for alias, col in extra_select]) + for alias, col in self.extra_select.items()]) aliases.extend(self.extra_select.keys()) self._select_aliases = dict.fromkeys(aliases) @@ -761,7 +758,7 @@ class Query(object): return for child in q_object.children: - if isinstance(child, tree.Node): + if isinstance(child, Node): self.where.start_subtree(q_object.connection) self.add_q(child) self.where.end_subtree() @@ -937,7 +934,7 @@ class Query(object): # level. self.distinct = False self.select = [select] - self.extra_select = {} + self.extra_select = SortedDict() def execute_sql(self, result_type=MULTI): """ diff --git a/docs/db-api.txt b/docs/db-api.txt index 9fc82add4a..9390cb0905 100644 --- a/docs/db-api.txt +++ b/docs/db-api.txt @@ -820,6 +820,34 @@ of the arguments is required, but you should use at least one of them. some database backends, such as some MySQL versions, don't support subqueries. + **New in Django development version** + In some rare cases, you might wish to pass parameters to the SQL fragments + in ``extra(select=...)```. Since the ``params`` attribute is a sequence + and the ``select`` attribute is a dictionary, some care is required so + that the parameters are matched up correctly with the extra select pieces. + Firstly, in this situation, you should use a + ``django.utils.datastructures.SortedDict`` for the ``select`` value, not + just a normal Python dictionary. Secondly, make sure that your parameters + for the ``select`` come first in the list and that you have not passed any + parameters to an earlier ``extra()`` call for this queryset. + + This will work:: + + Blog.objects.extra( + select=SortedDict(('a', '%s'), ('b', '%s')), + params=('one', 'two')) + + ... while this won't:: + + # Will not work! + Blog.objects.extra(where=['foo=%s'], params=('bar',)).extra( + select=SortedDict(('a', '%s'), ('b', '%s')), + params=('one', 'two')) + + In the second example, the earlier ``params`` usage will mess up the later + one. So always put your extra select pieces in the first ``extra()`` call + if you need to use parameters in them. + ``where`` / ``tables`` You can define explicit SQL ``WHERE`` clauses -- perhaps to perform non-explicit joins -- by using ``where``. You can manually add tables to diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index 0a7ae286b7..f0514dcee9 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -379,5 +379,20 @@ The all() method on querysets returns a copy of the queryset. >>> q1 = Item.objects.order_by('name') >>> id(q1) == id(q1.all()) False + +Bug #2902 +Parameters can be given to extra_select, *if* you use a SortedDict. + +(First we need to know which order the keys fall in "naturally" on your system, +so we can put things in the wrong way around from normal. A normal dict would +thus fail.) +>>> from django.utils.datastructures import SortedDict +>>> s = [('a', '%s'), ('b', '%s')] +>>> params = ['one', 'two'] +>>> if {'a': 1, 'b': 2}.keys() == ['a', 'b']: +... s.reverse() +... params.reverse() +>>> Item.objects.extra(select=SortedDict(s), params=params).values('a','b')[0] +{'a': u'one', 'b': u'two'} """}