diff options
| -rw-r--r-- | django/core/management/validation.py | 5 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 149 | ||||
| -rw-r--r-- | tests/modeltests/reserved_names/models.py | 2 | ||||
| -rw-r--r-- | tests/regressiontests/queries/models.py | 126 |
4 files changed, 233 insertions, 49 deletions
diff --git a/django/core/management/validation.py b/django/core/management/validation.py index 9e004d3d25..6bfc33b48d 100644 --- a/django/core/management/validation.py +++ b/django/core/management/validation.py @@ -190,7 +190,10 @@ def get_validation_errors(outfile, app=None): field_name = field_name[1:] if opts.order_with_respect_to and field_name == '_order': continue - if '.' in field_name: continue # Skip ordering in the format 'table.field'. + # Skip ordering in the format field1__field2 (FIXME: checking + # this format would be nice, but it's a little fiddly). + if '_' in field_name: + continue try: opts.get_field(field_name, many_to_many=False) except models.FieldDoesNotExist: diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 8914bce15c..35c6cc28dc 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -8,6 +8,7 @@ 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.db.models.sql.where import WhereNode, AND, OR @@ -35,7 +36,7 @@ GET_ITERATOR_CHUNK_SIZE = 100 # Separator used to split filter strings apart. LOOKUP_SEP = '__' -# Constants to make looking up tuple values clearerer. +# Constants to make looking up tuple values clearer. # Join lists TABLE_NAME = 0 RHS_ALIAS = 1 @@ -43,7 +44,7 @@ JOIN_TYPE = 2 LHS_ALIAS = 3 LHS_JOIN_COL = 4 RHS_JOIN_COL = 5 -# Alias maps lists +# Alias map lists ALIAS_TABLE = 0 ALIAS_REFCOUNT = 1 ALIAS_JOIN = 2 @@ -54,6 +55,8 @@ MULTI = 'multi' SINGLE = 'single' NONE = None +ORDER_PATTERN = re.compile(r'\?|[-+]?\w+$') + class Query(object): """ A single SQL query. @@ -91,6 +94,7 @@ class Query(object): self.extra_tables = [] self.extra_where = [] self.extra_params = [] + self.extra_order_by = [] def __str__(self): """ @@ -140,6 +144,7 @@ class Query(object): obj.extra_tables = self.extra_tables[:] obj.extra_where = self.extra_where[:] obj.extra_params = self.extra_params[:] + obj.extra_order_by = self.extra_order_by[:] obj.__dict__.update(kwargs) return obj @@ -189,19 +194,22 @@ class Query(object): in the query. """ self.pre_sql_setup() + out_cols = self.get_columns() + ordering = self.get_ordering() + # This must come after 'select' and 'ordering' -- see docstring of + # get_from_clause() for details. + from_, f_params = self.get_from_clause() + where, w_params = self.where.as_sql() + result = ['SELECT'] if self.distinct: result.append('DISTINCT') - out_cols = self.get_columns() result.append(', '.join(out_cols)) result.append('FROM') - from_, f_params = self.get_from_clause() result.extend(from_) params = list(f_params) - where, w_params = self.where.as_sql() - params.extend(w_params) if where: result.append('WHERE %s' % where) if self.extra_where: @@ -210,12 +218,12 @@ class Query(object): else: result.append('AND') result.append(' AND'.join(self.extra_where)) + params.extend(w_params) if self.group_by: grouping = self.get_grouping() result.append('GROUP BY %s' % ', '.join(grouping)) - ordering = self.get_ordering() if ordering: result.append('ORDER BY %s' % ', '.join(ordering)) @@ -312,11 +320,14 @@ class Query(object): # Ordering uses the 'rhs' ordering, unless it has none, in which case # the current ordering is used. self.order_by = rhs.order_by and rhs.order_by[:] or self.order_by + self.extra_order_by = (rhs.extra_order_by and rhs.extra_order_by[:] or + self.extra_order_by) def pre_sql_setup(self): """ - Does any necessary class setup prior to producing SQL. This is for - things that can't necessarily be done in __init__. + Does any necessary class setup immediately prior to producing SQL. This + is for things that can't necessarily be done in __init__ because we + might not have all the pieces in place at that time. """ if not self.tables: self.join((None, self.model._meta.db_table, None, None)) @@ -356,9 +367,14 @@ class Query(object): "FROM" part of the query, as well as any extra parameters that need to be included. Sub-classes, can override this to create a from-clause via a "select", for example (e.g. CountQuery). + + This should only be called after any SQL construction methods that + might change the tables we need. This means the select columns and + ordering must be done first. """ result = [] qn = self.quote_name_unless_alias + first = True for alias in self.tables: if not self.alias_map[alias][ALIAS_REFCOUNT]: continue @@ -370,12 +386,16 @@ class Query(object): % (join_type, qn(name), alias_str, qn(lhs), qn(lhs_col), qn(alias), qn(col))) else: - result.append('%s%s' % (qn(name), alias_str)) + connector = not first and ', ' or '' + result.append('%s%s%s' % (connector, qn(name), alias_str)) + first = False extra_tables = [] for t in self.extra_tables: alias, created = self.table_alias(t) if created: - result.append(', %s' % alias) + connector = not first and ', ' or '' + result.append('%s%s' % (connector, alias)) + first = False return result, [] def get_grouping(self): @@ -396,13 +416,20 @@ class Query(object): def get_ordering(self): """ Returns a tuple representing the SQL elements in the "order by" clause. + + Determining the ordering SQL can change the tables we need to include, + so this should be run *before* get_from_clause(). """ - if self.order_by is None: + if self.extra_order_by: + ordering = self.extra_order_by + elif self.order_by is None: ordering = [] else: + # Note that self.order_by can be empty in two ways: [] ("use the + # default"), which is handled here, and None ("no ordering"), which + # is handled in the previous test. ordering = self.order_by or self.model._meta.ordering qn = self.quote_name_unless_alias - opts = self.model._meta result = [] for field in ordering: if field == '?': @@ -416,24 +443,62 @@ class Query(object): order = 'ASC' result.append('%s %s' % (field, order)) continue - if field[0] == '-': - col = field[1:] - order = 'DESC' - else: - col = field - order = 'ASC' - if '.' in col: + if '.' in field: + # This came in through an extra(ordering=...) addition. Pass it + # on verbatim, after mapping the table name to an alias, if + # necessary. + col, order = get_order_dir(field) table, col = col.split('.', 1) - table = '%s.' % qn(self.table_alias(table)[0]) - elif col not in self.extra_select: - # Use the root model's database table as the referenced table. - table = '%s.' % qn(self.tables[0]) + result.append('%s.%s %s' % (qn(self.table_alias(table)[0]), col, + order)) + elif get_order_dir(field)[0] not in self.extra_select: + # 'col' is of the form 'field' or 'field1__field2' or + # 'field1__field2__field', etc. + for table, col, order in self.find_ordering_name(field, + self.model._meta): + result.append('%s.%s %s' % (qn(table), qn(col), order)) else: - table = '' - result.append('%s%s %s' % (table, - qn(orderfield_to_column(col, opts)), order)) + col, order = get_order_dir(field) + result.append('%s %s' % (qn(col), order)) return result + def find_ordering_name(self, name, opts, alias=None, default_order='ASC'): + """ + Returns the table alias (the name might not be unambiguous, the alias + will be) and column name for ordering by the given 'name' parameter. + The 'name' is of the form 'field1__field2__...__fieldN'. + """ + name, order = get_order_dir(name, default_order) + pieces = name.split(LOOKUP_SEP) + if not alias: + alias = self.join((None, opts.db_table, None, None)) + for elt in pieces: + joins, opts, unused1, field, col, unused2 = \ + self.get_next_join(elt, opts, alias, False) + if joins: + alias = joins[-1] + col = col or field.column + + # If we get to this point and the field is a relation to another model, + # append the default ordering for that model. + if joins and opts.ordering: + results = [] + for item in opts.ordering: + results.extend(self.find_ordering_name(item, opts, alias, + order)) + return results + + if alias: + # We have to do the same "final join" optimisation as in + # add_filter, since the final column might not otherwise be part of + # the select set (so we can't order on it). + join = self.alias_map[alias][ALIAS_JOIN] + if col == join[RHS_JOIN_COL]: + self.unref_alias(alias) + alias = join[LHS_ALIAS] + col = join[LHS_JOIN_COL] + return [(alias, col, order)] + def table_alias(self, table_name, create=False): """ Returns a table alias for the given table_name and whether this is a @@ -557,7 +622,7 @@ class Query(object): alias = self.join((root_alias, table, f.column, f.rel.get_related_field().column), exclusions=used) used.append(alias) - self.select.extend([(table, f2.column) + self.select.extend([(alias, f2.column) for f2 in f.rel.to._meta.fields]) self.fill_related_selections(f.rel.to._meta, alias, cur_depth + 1, used) @@ -802,6 +867,12 @@ class Query(object): possibly with a direction prefix ('-' or '?') -- or ordinals, corresponding to column positions in the 'select' list. """ + errors = [] + for item in ordering: + if not ORDER_PATTERN.match(item): + errors.append(item) + if errors: + raise TypeError('Invalid order_by arguments: %s' % errors) self.order_by.extend(ordering) def clear_ordering(self, force_empty=False): @@ -813,6 +884,7 @@ class Query(object): self.order_by = None else: self.order_by = [] + self.extra_order_by = [] def add_count_column(self): """ @@ -1043,6 +1115,9 @@ class CountQuery(Query): result, params = self._query.as_sql() return ['(%s) AS A1' % result], params + def get_ordering(self): + return () + def find_field(name, field_list, related_query): """ Finds a field with a specific name in a list of field instances. @@ -1077,14 +1152,16 @@ def get_legal_fields(opts): + field_choices(opts.get_all_related_objects(), True) + field_choices(opts.fields, False)) -def orderfield_to_column(name, opts): +def get_order_dir(field, default='ASC'): """ - For a field name specified in an "order by" clause, returns the database - column name. If 'name' is not a field in the current model, it is returned - unchanged. + Returns the field name and direction for an order specification. For + example, '-foo' is returned as ('foo', 'DESC'). + + The 'default' param is used to indicate which way no prefix (or a '+' + prefix) should sort. The '-' prefix always sorts the opposite way. """ - try: - return opts.get_field(name, False).column - except FieldDoesNotExist: - return name + dirn = {'ASC': ('ASC', 'DESC'), 'DESC': ('DESC', 'ASC')}[default] + if field[0] == '-': + return field[1:], dirn[1] + return field, dirn[0] diff --git a/tests/modeltests/reserved_names/models.py b/tests/modeltests/reserved_names/models.py index a11b8d9f88..f698b5bc49 100644 --- a/tests/modeltests/reserved_names/models.py +++ b/tests/modeltests/reserved_names/models.py @@ -45,8 +45,6 @@ h b >>> print v.where 2005-01-01 ->>> Thing.objects.order_by('select.when') -[<Thing: a>, <Thing: h>] >>> Thing.objects.dates('where', 'year') [datetime.datetime(2005, 1, 1, 0, 0), datetime.datetime(2006, 1, 1, 0, 0)] diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index 64baacc32d..c775772ada 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -1,5 +1,5 @@ """ -Various combination queries that have been problematic in the past. +Various complex queries that have been problematic in the past. """ from django.db import models @@ -12,9 +12,29 @@ class Tag(models.Model): def __unicode__(self): return self.name +class Note(models.Model): + note = models.CharField(maxlength=100) + + class Meta: + ordering = ['note'] + + def __unicode__(self): + return self.note + +class ExtraInfo(models.Model): + info = models.CharField(maxlength=100) + note = models.ForeignKey(Note) + + class Meta: + ordering = ['info'] + + def __unicode__(self): + return self.info + class Author(models.Model): name = models.CharField(maxlength=10) num = models.IntegerField(unique=True) + extra = models.ForeignKey(ExtraInfo) def __unicode__(self): return self.name @@ -23,6 +43,10 @@ class Item(models.Model): name = models.CharField(maxlength=10) tags = models.ManyToManyField(Tag, blank=True, null=True) creator = models.ForeignKey(Author) + note = models.ForeignKey(Note) + + class Meta: + ordering = ['-note', 'name'] def __unicode__(self): return self.name @@ -34,6 +58,26 @@ class Report(models.Model): def __unicode__(self): return self.name +class Ranking(models.Model): + rank = models.IntegerField() + author = models.ForeignKey(Author) + + class Meta: + # A complex ordering specification. Should stress the system a bit. + ordering = ('author__extra__note', 'author__name', 'rank') + + def __unicode__(self): + return '%d: %s' % (self.rank, self.author.name) + +class Cover(models.Model): + title = models.CharField(maxlength=50) + item = models.ForeignKey(Item) + + class Meta: + ordering = ['item'] + + def __unicode__(self): + return self.title __test__ = {'API_TESTS':""" >>> t1 = Tag(name='t1') @@ -47,25 +91,38 @@ __test__ = {'API_TESTS':""" >>> t5 = Tag(name='t5', parent=t3) >>> t5.save() +>>> n1 = Note(note='n1') +>>> n1.save() +>>> n2 = Note(note='n2') +>>> n2.save() +>>> n3 = Note(note='n3') +>>> n3.save() ->>> a1 = Author(name='a1', num=1001) +Create these out of order so that sorting by 'id' will be different to sorting +by 'info'. Helps detect some problems later. +>>> e2 = ExtraInfo(info='e2', note=n2) +>>> e2.save() +>>> e1 = ExtraInfo(info='e1', note=n1) +>>> e1.save() + +>>> a1 = Author(name='a1', num=1001, extra=e1) >>> a1.save() ->>> a2 = Author(name='a2', num=2002) +>>> a2 = Author(name='a2', num=2002, extra=e1) >>> a2.save() ->>> a3 = Author(name='a3', num=3003) +>>> a3 = Author(name='a3', num=3003, extra=e2) >>> a3.save() ->>> a4 = Author(name='a4', num=4004) +>>> a4 = Author(name='a4', num=4004, extra=e2) >>> a4.save() ->>> i1 = Item(name='one', creator=a1) +>>> i1 = Item(name='one', creator=a1, note=n3) >>> i1.save() >>> i1.tags = [t1, t2] ->>> i2 = Item(name='two', creator=a2) +>>> i2 = Item(name='two', creator=a2, note=n2) >>> i2.save() >>> i2.tags = [t1, t3] ->>> i3 = Item(name='three', creator=a2) +>>> i3 = Item(name='three', creator=a2, note=n3) >>> i3.save() ->>> i4 = Item(name='four', creator=a4) +>>> i4 = Item(name='four', creator=a4, note=n3) >>> i4.save() >>> i4.tags = [t4] @@ -74,6 +131,20 @@ __test__ = {'API_TESTS':""" >>> r2 = Report(name='r2', creator=a3) >>> r2.save() +Ordering by 'rank' gives us rank2, rank1, rank3. Ordering by the Meta.ordering +will be rank3, rank2, rank1. +>>> rank1 = Ranking(rank=2, author=a2) +>>> rank1.save() +>>> rank2 = Ranking(rank=1, author=a3) +>>> rank2.save() +>>> rank3 = Ranking(rank=3, author=a1) +>>> rank3.save() + +>>> c1 = Cover(title="first", item=i4) +>>> c1.save() +>>> c2 = Cover(title="second", item=i2) +>>> c2.save() + Bug #1050 >>> Item.objects.filter(tags__isnull=True) [<Item: three>] @@ -119,7 +190,7 @@ Bug #1878, #2939 # Create something with a duplicate 'name' so that we can test multi-column # cases (which require some tricky SQL transformations under the covers). ->>> xx = Item(name='four', creator=a2) +>>> xx = Item(name='four', creator=a2, note=n1) >>> xx.save() >>> Item.objects.exclude(name='two').values('creator', 'name').distinct().count() 4 @@ -206,5 +277,40 @@ Bug #2400 Bug #2496 >>> Item.objects.extra(tables=['queries_author']).select_related().order_by('name')[:1] [<Item: four>] + +Bug #2076 +# Ordering on related tables should be possible, even if the table is not +# otherwise involved. +>>> Item.objects.order_by('note__note', 'name') +[<Item: two>, <Item: four>, <Item: one>, <Item: three>] + +# Ordering on a related field should use the remote model's default ordering as +# a final step. +>>> Author.objects.order_by('extra', '-name') +[<Author: a2>, <Author: a1>, <Author: a4>, <Author: a3>] + +# If the remote model does not have a default ordering, we order by its 'id' +# field. +>>> Item.objects.order_by('creator', 'name') +[<Item: one>, <Item: three>, <Item: two>, <Item: four>] + +# Cross model ordering is possible in Meta, too. +>>> Ranking.objects.all() +[<Ranking: 3: a1>, <Ranking: 2: a2>, <Ranking: 1: a3>] +>>> Ranking.objects.all().order_by('rank') +[<Ranking: 1: a3>, <Ranking: 2: a2>, <Ranking: 3: a1>] + +>>> Cover.objects.all() +[<Cover: first>, <Cover: second>] + +Bugs #2874, #3002 +>>> qs = Item.objects.select_related().order_by('note__note', 'name') +>>> list(qs) +[<Item: two>, <Item: four>, <Item: one>, <Item: three>] + +# This is also a good select_related() test because there are multiple Note +# entries in the SQL. The two Note items should be different. +>>> qs[0].note, qs[0].creator.extra.note +(<Note: n2>, <Note: n1>) """} |