Ticket #16187: 16187_uqly.diff

django/db/models/sql/query.py

@@ -26 +26 @@
 
 __all__ = ['Query', 'RawQuery']
 
+FOLLOW_FORWARD = 'FORWARD'
+FOLLOW_REVERSE = 'REVERSE'
+
 class RawQuery(object):
     """
     A single raw SQL query
@@ -1014 +1017 @@
         # Add the aggregate to the query
         aggregate.add_to_query(self, alias, col=col, source=source, is_summary=is_summary)
 
+    def get_field_by_name(self, opts, filter_name, allow_explicit_fk=True):
+        """
+        A wrapper to opts.get_field_by_name, so that 'foo_id' -> 'foo'
+        translation does not clutter the main code. TODO: Move to Options.
+        """
+        try:
+            field, model, direct, m2m = opts.get_field_by_name(filter_name)
+        except FieldDoesNotExist:
+            for f in opts.fields:
+                if allow_explicit_fk and filter_name == f.attname:
+                    # XXX: A hack to allow foo_id to work in values() for
+                    # backwards compatibility purposes. If we dropped that
+                    # feature, this could be removed.
+                    field, model, direct, m2m = opts.get_field_by_name(f.name)
+                    break
+            else:
+                names = opts.get_all_field_names() + self.aggregate_select.keys()
+                raise FieldError("Cannot resolve keyword %r into field. "
+                        "Choices are: %s" % (filter_name, ", ".join(names)))
+        return field, model, direct, m2m
+
+    def filter_chain_to_relations(self, filter_chain):
+        """
+        Turns the passed in filter_chain (for example [related_model, id])
+        into relations and final_field.
+
+        The relations will be in the format
+            (field, direction)
+        Direction itself is either FOLLOW_FORWARD or FOLLOW_REVERSE
+        The format is slightly inconvenient in that the direct flag informs if
+        the field lives in the related model or in the previous model of the
+        join chain. The fields in relations list will always be ForeignKeys, 
+        OneToOneKeys or GenericForeignKeys(?).
+
+        The final field will be the last field in the chain.
+
+        Note that one filter_chain part can lead to multiple relations due to
+        model inheritance and many_to_many filtering.
+        """
+        relations, final_field = [], None
+        opts = self.model._meta
+        field = None
+        for pos, filter_name in enumerate(filter_chain):
+            if filter_name == 'pk':
+                filter_name = opts.pk.name
+            try:
+                field, model, direct, m2m = self.get_field_by_name(opts, filter_name)
+            except FieldError:
+                # We have reached the lookup part of the filter chain
+                if field is None:
+                    # We didn't resolve a single field.
+                    raise
+                return relations, field, filter_chain[pos:] 
+            if model:
+                # The field lives on a base class of the current model.
+                # Skip the chain of proxy to the concrete proxied model
+                proxied_model = get_proxied_model(opts)
+                # Process the internal one-to-one chain into
+                # relations
+                for int_model in opts.get_base_chain(model):
+                    if int_model is proxied_model:
+                        # skip proxy models
+                        opts = int_model._meta
+                        continue
+                    # Note that we do not care that there might be unecessary
+                    # relations here (stepping through intermediatry relations to get
+                    # to a field in the base chain, even though the field's value is
+                    # guaranteed to stay the same throughout the chain). We can prune
+                    # them later on if necessary.
+                    o2o_field = opts.get_ancestor_link(int_model)
+                    relations.append((o2o_field, FOLLOW_FORWARD))
+                    opts = int_model._meta
+            if m2m and (direct and field.rel.through):
+                # For some strange reason GenericRelations pretend to be
+                # m2m fields, although they are not. The above field.rel.through
+                # check ensures we do not end up here: proper fix - refactor
+                # generic relations
+                if not direct:
+                    # This nice little line fetches related model's field
+                    field = field.field
+                through_model = field.rel.through
+                through_opts = through_model._meta
+                through_field1_name = field.m2m_field_name()
+                field1, _, _, _ = through_opts.get_field_by_name(through_field1_name)
+                through_field2_name = field.m2m_reverse_field_name()
+                field2, _, _, _ = through_opts.get_field_by_name(through_field2_name)
+                if direct:
+                    relations.append((field1, FOLLOW_FORWARD))
+                    relations.append((field2, FOLLOW_FORWARD))
+                    opts = field.rel.to._meta
+                else:
+                    relations.append((field2, FOLLOW_REVERSE))
+                    relations.append((field1, FOLLOW_REVERSE))
+                    opts = field.opts
+            else:
+                if direct and field.rel:
+                    # Local foreign key
+                    relations.append((field, FOLLOW_FORWARD))
+                    opts = field.rel.to._meta
+                elif direct:
+                    # This is a local field which is not a relation -> the final field.
+                    return relations, field, filter_chain[pos+1:]
+                else:
+                    # ForeignKey or OneToOneKey defined in the target model.
+                    field = field.field
+                    relations.append((field, FOLLOW_REVERSE))
+                    opts = field.opts
+
+        # We end up in this case when querying 'related_field=val'
+        return relations, field, []
+
     def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
             can_reuse=None, process_extras=True, force_having=False):
         """
@@ -1040 +1154 @@
         joining process will be processed. This parameter is set to False
         during the processing of extra filters to avoid infinite recursion.
         """
-        arg, value = filter_expr
-        parts = arg.split(LOOKUP_SEP)
+        filter_path, value = filter_expr
+        parts = filter_path.split(LOOKUP_SEP)
         if not parts:
-            raise FieldError("Cannot parse keyword query %r" % arg)
+            raise FieldError("Cannot parse keyword query %r" % filter_path)
+        
+        aggregate_lookup = None
+        lookup_type = 'exact'
+        for alias, aggregate in self.aggregates.items():
+            if alias == LOOKUP_SEP.join(parts[0:-1]):
+                 aggregate_lookup = aggregate
+                 lookup_type = parts[-1]
+                 # aggregates could support custom lookups, too.
+                 if lookup_type not in self.query_terms:
+                     raise Exception('TODO: correct exception type and message')
+                 break
+            if alias == filter_path:
+                 aggregate_lookup = aggregate
+                 break
+
+        if not aggregate_lookup:
+            _ , final_field, lookup_parts = self.filter_chain_to_relations(parts)
+            # we could use the final field and lookup_parts for doing custom lookups
+            # not done yet.
+            if len(lookup_parts) == 1:
+                if lookup_parts[0] not in self.query_terms:
+                    raise FieldError(
+                        "Join on field %r not permitted. "
+                        "Did you misspell %r for the lookup type?"
+                        % (final_field.name, lookup_parts[0])
+                    )
+                lookup_type = lookup_parts[0]
+                parts = parts[0:-1]
+            if len(lookup_parts) > 1:
+                if not hasattr(final_field, 'rel'):
+                    raise FieldError("Join on field %r not permitted." % final_field.name)
+                else:
+                    names = (final_field.rel.to._meta.get_all_field_names() +
+                             self.aggregate_select.keys())
+                    raise FieldError("Cannot resolve keyword %r into field. "
+                          "Choices are: %s" % (lookup_parts[0], ", ".join(names)))
 
-        # Work out the lookup type and remove it from 'parts', if necessary.
-        if len(parts) == 1 or parts[-1] not in self.query_terms:
+        if lookup_type == None:
             lookup_type = 'exact'
-        else:
-            lookup_type = parts.pop()
 
         # By default, this is a WHERE clause. If an aggregate is referenced
         # in the value, the filter will be promoted to a HAVING
@@ -1069 +1216 @@
             value = SQLEvaluator(value, self)
             having_clause = value.contains_aggregate
 
-        for alias, aggregate in self.aggregates.items():
-            if alias in (parts[0], LOOKUP_SEP.join(parts)):
-                entry = self.where_class()
-                entry.add((aggregate, lookup_type, value), AND)
-                if negate:
-                    entry.negate()
-                self.having.add(entry, connector)
-                return
+        if aggregate_lookup:
+            entry = self.where_class()
+            entry.add((aggregate, lookup_type, value), AND)
+            if negate:
+                entry.negate()
+            self.having.add(entry, connector)
+            return
+
 
         opts = self.get_meta()
         alias = self.get_initial_alias()

tests/modeltests/aggregation/tests.py

@@ -437 +437 @@
         vals = Author.objects.filter(pk=1).aggregate(Count("friends__id"))
         self.assertEqual(vals, {"friends__id__count": 2})
 
-        books = Book.objects.annotate(num_authors=Count("authors__name")).filter(num_authors__ge=2).order_by("pk")
+        books = Book.objects.annotate(num_authors=Count("authors__name")).filter(num_authors=2).order_by("pk")
         self.assertQuerysetEqual(
             books, [
                 "The Definitive Guide to Django: Web Development Done Right",

tests/modeltests/lookup/tests.py

@@ -469 +469 @@
             self.assertEqual(str(ex), "Cannot resolve keyword 'pub_date_year' "
                              "into field. Choices are: author, headline, id, pub_date, tag")
         try:
+            Article.objects.filter(author__foo__bar=1)
+            self.fail('FieldError not raised')
+        except FieldError, ex:
+            self.assertEqual(str(ex), "Cannot resolve keyword 'foo' "
+                             "into field. Choices are: article, id, name")
+        try:
             Article.objects.filter(headline__starts='Article')
             self.fail('FieldError not raised')
         except FieldError, ex:
             self.assertEqual(str(ex), "Join on field 'headline' not permitted. "
                              "Did you misspell 'starts' for the lookup type?")
 
+
     def test_regex(self):
         # Create some articles with a bit more interesting headlines for testing field lookups:
         for a in Article.objects.all():

tests/regressiontests/null_queries/tests.py

@@ -40 +40 @@
         self.assertRaises(ValueError, Choice.objects.filter, id__gt=None)
 
         # Can't use None on anything other than __exact
-        self.assertRaises(ValueError, Choice.objects.filter, foo__gt=None)
+        self.assertRaises(FieldError, Choice.objects.filter, foo__gt=None)
 
         # Related managers use __exact=None implicitly if the object hasn't been saved.
         p2 = Poll(question="How?")