diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index 61182a6..a05c352 100644 --- a/django/contrib/admin/util.py +++ b/django/contrib/admin/util.py @@ -12,19 +12,6 @@ from django.utils.encoding import force_unicode, smart_unicode, smart_str from django.utils.translation import ungettext from django.core.urlresolvers import reverse -def lookup_needs_distinct(opts, lookup_path): - """ - Returns True if 'distinct()' should be used to query the given lookup path. - """ - field_name = lookup_path.split('__', 1)[0] - field = opts.get_field_by_name(field_name)[0] - if ((hasattr(field, 'rel') and - isinstance(field.rel, models.ManyToManyRel)) or - (isinstance(field, models.related.RelatedObject) and - not field.field.unique)): - return True - return False - def prepare_lookup_value(key, value): """ Returns a lookup value prepared to be used in queryset filtering. diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index b11f9d5..ef8db01 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -12,7 +12,7 @@ from django.utils.http import urlencode from django.contrib.admin import FieldListFilter from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.util import (quote, get_fields_from_path, - lookup_needs_distinct, prepare_lookup_value) + prepare_lookup_value) # Changelist settings ALL_VAR = 'all' @@ -79,7 +79,6 @@ class ChangeList(object): def get_filters(self, request): lookup_params = self.params.copy() # a dictionary of the query string - use_distinct = False # Remove all the parameters that are globally and systematically # ignored. @@ -120,25 +119,20 @@ class ChangeList(object): field = get_fields_from_path(self.model, field_path)[-1] spec = field_list_filter_class(field, request, lookup_params, self.model, self.model_admin, field_path=field_path) - # Check if we need to use distinct() - use_distinct = (use_distinct or - lookup_needs_distinct(self.lookup_opts, - field_path)) + if spec and spec.has_output(): filter_specs.append(spec) # At this point, all the parameters used by the various ListFilters # have been removed from lookup_params, which now only contains other # parameters passed via the query string. We now loop through the - # remaining parameters both to ensure that all the parameters are valid - # fields and to determine if at least one of them needs distinct(). If - # the lookup parameters aren't real fields, then bail out. + # remaining parameters to ensure that all the parameters are valid + # fields. If the lookup parameters aren't real fields, then bail out. try: for key, value in lookup_params.items(): lookup_params[key] = prepare_lookup_value(key, value) - use_distinct = (use_distinct or - lookup_needs_distinct(self.lookup_opts, key)) - return filter_specs, bool(filter_specs), lookup_params, use_distinct + + return filter_specs, bool(filter_specs), lookup_params except FieldDoesNotExist, e: raise IncorrectLookupParameters(e) @@ -298,7 +292,7 @@ class ChangeList(object): def get_query_set(self, request): # First, we collect all the declared list filters. (self.filter_specs, self.has_filters, remaining_lookup_params, - use_distinct) = self.get_filters(request) + ) = self.get_filters(request) # Then, we let every list filter modify the queryset to its liking. qs = self.root_query_set @@ -363,13 +357,8 @@ class ChangeList(object): or_queries = [models.Q(**{orm_lookup: bit}) for orm_lookup in orm_lookups] qs = qs.filter(reduce(operator.or_, or_queries)) - if not use_distinct: - for search_spec in orm_lookups: - if lookup_needs_distinct(self.lookup_opts, search_spec): - use_distinct = True - break - if use_distinct: + if qs.query.contains_multijoin: return qs.distinct() else: return qs diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index ce11716..992b105 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -155,6 +155,10 @@ class Query(object): # are the fields to defer, or False if these are the only fields to # load. self.deferred_loading = (set(), True) + # We keep track if the query contains multijoins (reverse FK joins or + # m2m joins) so that users of the queryset can easily decide if they + # need to use distinct or not + self.contains_multijoin = False def __str__(self): """ @@ -300,7 +304,7 @@ class Query(object): else: obj.used_aliases = set() obj.filter_is_sticky = False - + obj.contains_multijoin = self.contains_multijoin obj.__dict__.update(kwargs) if hasattr(obj, '_setup_query'): obj._setup_query() @@ -1315,10 +1319,15 @@ class Query(object): raise FieldError("Cannot resolve keyword %r into field. " "Choices are: %s" % (name, ", ".join(names))) - if not allow_many and (m2m or not direct): - for alias in joins: - self.unref_alias(alias) - raise MultiJoin(pos + 1) + if m2m or not direct: + if not allow_many: + for alias in joins: + self.unref_alias(alias) + raise MultiJoin(pos + 1) + # This is a heuristic only - it is possible the query does not + # actually contain a multijoin due to filtering reducing the + # join to singlevalued join. + self.contains_multijoin = True if model: # The field lives on a base class of the current model. # Skip the chain of proxy to the concrete proxied model