From c81d69354ab44e9b12807af9d544d77149b8ea73 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sat, 1 Jul 2006 01:14:41 +0000 Subject: [PATCH] Fixed #2217 -- Allowed raw objects to be used in __exact and __in query terms. Existing use of primary keys in query terms is preserved. git-svn-id: http://code.djangoproject.com/svn/django/trunk@3246 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/fields/related.py | 26 ++++++++++++++ django/db/models/query.py | 48 ++++++++++++++++++++----- django/db/models/related.py | 4 +++ tests/modeltests/many_to_many/models.py | 23 +++++++++++- tests/modeltests/many_to_one/models.py | 28 ++++++++++++--- tests/modeltests/one_to_one/models.py | 24 +++++++++++++ 6 files changed, 139 insertions(+), 14 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 5ee1aec022..9a8a61878e 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -78,6 +78,32 @@ class RelatedField(object): related = RelatedObject(other, cls, self) self.contribute_to_related_class(other, related) + def get_db_prep_lookup(self, lookup_type, value): + # If we are doing a lookup on a Related Field, we must be + # comparing object instances. The value should be the PK of value, + # not value itself. + def pk_trace(value): + # Value may be a primary key, or an object held in a relation. + # If it is an object, then we need to get the primary key value for + # that object. In certain conditions (especially one-to-one relations), + # the primary key may itself be an object - so we need to keep drilling + # down until we hit a value that can be used for a comparison. + v = value + try: + while True: + v = getattr(v, v._meta.pk.name) + except AttributeError: + pass + return v + + if lookup_type == 'exact': + return [pk_trace(value)] + if lookup_type == 'in': + return [pk_trace(v) for v in value] + elif lookup_type == 'isnull': + return [] + raise TypeError, "Related Field has invalid lookup: %s" % lookup_type + def _get_related_query_name(self, opts): # This method defines the name that can be used to identify this related object # in a table-spanning query. It uses the lower-cased object_name by default, diff --git a/django/db/models/query.py b/django/db/models/query.py index efbe68cb66..aa2138643e 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -841,12 +841,14 @@ def lookup_inner(path, clause, value, opts, table, column): ) if path: + # There are elements left in the path. More joins are required. if len(path) == 1 and path[0] in (new_opts.pk.name, None) \ and clause in ('exact', 'isnull') and not join_required: - # If the last name query is for a key, and the search is for - # isnull/exact, then the current (for N-1) or intermediate - # (for N-N) table can be used for the search - no need to join an - # extra table just to check the primary key. + # If the next and final name query is for a primary key, + # and the search is for isnull/exact, then the current + # (for N-1) or intermediate (for N-N) table can be used + # for the search - no need to join an extra table just + # to check the primary key. new_table = current_table else: # There are 1 or more name queries pending, and we have ruled out @@ -872,13 +874,41 @@ def lookup_inner(path, clause, value, opts, table, column): where.extend(where2) params.extend(params2) else: - # Evaluate clause on current table. - if name in (current_opts.pk.name, None) and clause in ('exact', 'isnull') and current_column: - # If this is an exact/isnull key search, and the last pass - # found/introduced a current/intermediate table that we can use to - # optimize the query, then use that column name. + # No elements left in path. Current element is the element on which + # the search is being performed. + + if join_required: + # Last query term is a RelatedObject + if field.field.rel.multiple: + # RelatedObject is from a 1-N relation. + # Join is required; query operates on joined table. + column = new_opts.pk.name + joins[backend.quote_name(new_table)] = ( + backend.quote_name(new_opts.db_table), + "INNER JOIN", + "%s.%s = %s.%s" % + (backend.quote_name(current_table), + backend.quote_name(join_column), + backend.quote_name(new_table), + backend.quote_name(new_column)) + ) + current_table = new_table + else: + # RelatedObject is from a 1-1 relation, + # No need to join; get the pk value from the related object, + # and compare using that. + column = current_opts.pk.name + elif intermediate_table: + # Last query term is a related object from an N-N relation. + # Join from intermediate table is sufficient. + column = join_column + elif name == current_opts.pk.name and clause in ('exact', 'isnull') and current_column: + # Last query term is for a primary key. If previous iterations + # introduced a current/intermediate table that can be used to + # optimize the query, then use that table and column name. column = current_column else: + # Last query term was a normal field. column = field.column where.append(get_where_clause(clause, current_table + '.', column, value)) diff --git a/django/db/models/related.py b/django/db/models/related.py index 4ab8cde5e7..ee3b916cf4 100644 --- a/django/db/models/related.py +++ b/django/db/models/related.py @@ -70,6 +70,10 @@ class RelatedObject(object): else: return [None] * self.field.rel.num_in_admin + def get_db_prep_lookup(self, lookup_type, value): + # Defer to the actual field definition for db prep + return self.field.get_db_prep_lookup(lookup_type, value) + def editable_fields(self): "Get the fields in this class that should be edited inline." return [f for f in self.opts.fields + self.opts.many_to_many if f.editable and f != self.field] diff --git a/tests/modeltests/many_to_many/models.py b/tests/modeltests/many_to_many/models.py index e80afece46..0e989a0fbe 100644 --- a/tests/modeltests/many_to_many/models.py +++ b/tests/modeltests/many_to_many/models.py @@ -75,6 +75,10 @@ API_TESTS = """ [, ] >>> Article.objects.filter(publications__pk=1) [, ] +>>> Article.objects.filter(publications=1) +[, ] +>>> Article.objects.filter(publications=p1) +[, ] >>> Article.objects.filter(publications__title__startswith="Science") [, ] @@ -89,6 +93,13 @@ API_TESTS = """ >>> Article.objects.filter(publications__title__startswith="Science").distinct().count() 1 +>>> Article.objects.filter(publications__in=[1,2]).distinct() +[, ] +>>> Article.objects.filter(publications__in=[1,p2]).distinct() +[, ] +>>> Article.objects.filter(publications__in=[p1,p2]).distinct() +[, ] + # Reverse m2m queries are supported (i.e., starting at the table that doesn't # have a ManyToManyField). >>> Publication.objects.filter(id__exact=1) @@ -101,9 +112,19 @@ API_TESTS = """ >>> Publication.objects.filter(article__id__exact=1) [] - >>> Publication.objects.filter(article__pk=1) [] +>>> Publication.objects.filter(article=1) +[] +>>> Publication.objects.filter(article=a1) +[] + +>>> Publication.objects.filter(article__in=[1,2]).distinct() +[, , , ] +>>> Publication.objects.filter(article__in=[1,a2]).distinct() +[, , , ] +>>> Publication.objects.filter(article__in=[a1,a2]).distinct() +[, , , ] # If we delete a Publication, its Articles won't be able to access it. >>> p1.delete() diff --git a/tests/modeltests/many_to_one/models.py b/tests/modeltests/many_to_one/models.py index a830ffbdc2..b7d27e2ed3 100644 --- a/tests/modeltests/many_to_one/models.py +++ b/tests/modeltests/many_to_one/models.py @@ -151,10 +151,20 @@ False [, ] # Find all Articles for the Reporter whose ID is 1. +# Use direct ID check, pk check, and object comparison >>> Article.objects.filter(reporter__id__exact=1) [, ] >>> Article.objects.filter(reporter__pk=1) [, ] +>>> Article.objects.filter(reporter=1) +[, ] +>>> Article.objects.filter(reporter=r) +[, ] + +>>> Article.objects.filter(reporter__in=[1,2]).distinct() +[, , ] +>>> Article.objects.filter(reporter__in=[r,r2]).distinct() +[, , ] # You need two underscores between "reporter" and "id" -- not one. >>> Article.objects.filter(reporter_id__exact=1) @@ -168,10 +178,6 @@ Traceback (most recent call last): ... TypeError: Cannot resolve keyword 'reporter_id' into field -# "pk" shortcut syntax works in a related context, too. ->>> Article.objects.filter(reporter__pk=1) -[, ] - # You can also instantiate an Article by passing # the Reporter's ID instead of a Reporter object. >>> a3 = Article(id=None, headline="This is a test", pub_date=datetime(2005, 7, 27), reporter_id=r.id) @@ -200,6 +206,18 @@ TypeError: Cannot resolve keyword 'reporter_id' into field [] >>> Reporter.objects.filter(article__pk=1) [] +>>> Reporter.objects.filter(article=1) +[] +>>> Reporter.objects.filter(article=a) +[] + +>>> Reporter.objects.filter(article__in=[1,4]).distinct() +[] +>>> Reporter.objects.filter(article__in=[1,a3]).distinct() +[] +>>> Reporter.objects.filter(article__in=[a,a3]).distinct() +[] + >>> Reporter.objects.filter(article__headline__startswith='This') [, , ] >>> Reporter.objects.filter(article__headline__startswith='This').distinct() @@ -216,6 +234,8 @@ TypeError: Cannot resolve keyword 'reporter_id' into field [, , , ] >>> Reporter.objects.filter(article__reporter__first_name__startswith='John').distinct() [] +>>> Reporter.objects.filter(article__reporter__exact=r).distinct() +[] # If you delete a reporter, his articles will be deleted. >>> Article.objects.all() diff --git a/tests/modeltests/one_to_one/models.py b/tests/modeltests/one_to_one/models.py index 27c16b5a34..f95556c08d 100644 --- a/tests/modeltests/one_to_one/models.py +++ b/tests/modeltests/one_to_one/models.py @@ -94,6 +94,12 @@ DoesNotExist: Restaurant matching query does not exist. >>> Restaurant.objects.get(place__exact=1) +>>> Restaurant.objects.get(place__exact=p1) + +>>> Restaurant.objects.get(place=1) + +>>> Restaurant.objects.get(place=p1) + >>> Restaurant.objects.get(place__pk=1) >>> Restaurant.objects.get(place__name__startswith="Demon") @@ -105,8 +111,18 @@ DoesNotExist: Restaurant matching query does not exist. >>> Place.objects.get(restaurant__place__exact=1) +>>> Place.objects.get(restaurant__place__exact=p1) + >>> Place.objects.get(restaurant__pk=1) +>>> Place.objects.get(restaurant=1) + +>>> Place.objects.get(restaurant=r) + +>>> Place.objects.get(restaurant__exact=1) + +>>> Place.objects.get(restaurant__exact=r) + # Add a Waiter to the Restaurant. >>> w = r.waiter_set.create(name='Joe') @@ -115,14 +131,22 @@ DoesNotExist: Restaurant matching query does not exist. # Query the waiters +>>> Waiter.objects.filter(restaurant__place__pk=1) +[] >>> Waiter.objects.filter(restaurant__place__exact=1) [] +>>> Waiter.objects.filter(restaurant__place__exact=p1) +[] >>> Waiter.objects.filter(restaurant__pk=1) [] >>> Waiter.objects.filter(id__exact=1) [] >>> Waiter.objects.filter(pk=1) [] +>>> Waiter.objects.filter(restaurant=1) +[] +>>> Waiter.objects.filter(restaurant=r) +[] # Delete the restaurant; the waiter should also be removed >>> r = Restaurant.objects.get(pk=1)