Ticket #15819: query_contains_multijoin.diff

django/contrib/admin/util.py

@@ -12 +12 @@
 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.

django/contrib/admin/views/main.py

@@ -12 +12 @@
 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 +79 @@
 
     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 +119 @@
                         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 +292 @@
     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 +357 @@
                 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

django/db/models/sql/query.py

@@ -155 +155 @@
         # 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 +304 @@
         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 +1319 @@
                     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