diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index f5f27940b5..b09412aa29 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -376,7 +376,7 @@ def create_many_related_manager(superclass, through=False): raise ValueError("%r instance needs to have a primary key value before a many-to-many relationship can be used." % instance.__class__.__name__) def get_query_set(self): - return superclass.get_query_set(self).filter(**(self.core_filters)) + return superclass.get_query_set(self)._next_is_sticky().filter(**(self.core_filters)) # If the ManyToMany relation has an intermediary model, # the add and remove methods do not exist. diff --git a/django/db/models/query.py b/django/db/models/query.py index d7180bbc1f..da300a5dd4 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -121,6 +121,7 @@ class QuerySet(object): self.query = query or sql.Query(self.model, connection) self._result_cache = None self._iter = None + self._sticky_filter = False ######################## # PYTHON MAGIC METHODS # @@ -589,7 +590,10 @@ class QuerySet(object): def _clone(self, klass=None, setup=False, **kwargs): if klass is None: klass = self.__class__ - c = klass(model=self.model, query=self.query.clone()) + query = self.query.clone() + if self._sticky_filter: + query.filter_is_sticky = True + c = klass(model=self.model, query=query) c.__dict__.update(kwargs) if setup and hasattr(c, '_setup_query'): c._setup_query() @@ -607,6 +611,17 @@ class QuerySet(object): except StopIteration: self._iter = None + def _next_is_sticky(self): + """ + Indicates that the next filter call and the one following that should + be treated as a single filter. This is only important when it comes to + determining when to reuse tables for many-to-many filters. Required so + that we can filter naturally on the results of related managers. + """ + obj = self._clone() + obj._sticky_filter = True + return obj + def _merge_sanity_check(self, other): """ Checks that we are merging two comparable QuerySet classes. By default diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 8cf3f58105..287890a63e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -58,6 +58,8 @@ class Query(object): self.select_fields = [] self.related_select_fields = [] self.dupe_avoidance = {} + self.used_aliases = set() + self.filter_is_sticky = False # SQL-related attributes self.select = [] @@ -78,7 +80,7 @@ class Query(object): # These are for extensions. The contents are more or less appended # verbatim to the appropriate clause. - self.extra_select = SortedDict() # Maps col_alias -> col_sql. + self.extra_select = SortedDict() # Maps col_alias -> (col_sql, params). self.extra_tables = () self.extra_where = () self.extra_params = () @@ -185,6 +187,11 @@ class Query(object): obj.extra_where = self.extra_where obj.extra_params = self.extra_params obj.extra_order_by = self.extra_order_by + if self.filter_is_sticky and self.used_aliases: + obj.used_aliases = self.used_aliases.copy() + else: + obj.used_aliases = set() + obj.filter_is_sticky = False obj.__dict__.update(kwargs) if hasattr(obj, '_setup_query'): obj._setup_query() @@ -1148,31 +1155,32 @@ class Query(object): Can also be used to add anything that has an 'add_to_query()' method. """ if used_aliases is None: - used_aliases = set() + used_aliases = self.used_aliases if hasattr(q_object, 'add_to_query'): # Complex custom objects are responsible for adding themselves. q_object.add_to_query(self, used_aliases) - return - - if self.where and q_object.connector != AND and len(q_object) > 1: - self.where.start_subtree(AND) - subtree = True else: - subtree = False - connector = AND - for child in q_object.children: - if isinstance(child, Node): - self.where.start_subtree(connector) - self.add_q(child, used_aliases) - self.where.end_subtree() + if self.where and q_object.connector != AND and len(q_object) > 1: + self.where.start_subtree(AND) + subtree = True else: - self.add_filter(child, connector, q_object.negated, - can_reuse=used_aliases) - connector = q_object.connector - if q_object.negated: - self.where.negate() - if subtree: - self.where.end_subtree() + subtree = False + connector = AND + for child in q_object.children: + if isinstance(child, Node): + self.where.start_subtree(connector) + self.add_q(child, used_aliases) + self.where.end_subtree() + else: + self.add_filter(child, connector, q_object.negated, + can_reuse=used_aliases) + connector = q_object.connector + if q_object.negated: + self.where.negate() + if subtree: + self.where.end_subtree() + if self.filter_is_sticky: + self.used_aliases = used_aliases def setup_joins(self, names, opts, alias, dupe_multis, allow_many=True, allow_explicit_fk=False, can_reuse=None): diff --git a/tests/regressiontests/m2m_through_regress/models.py b/tests/regressiontests/m2m_through_regress/models.py index 16a3ab2ce3..3775c7e158 100644 --- a/tests/regressiontests/m2m_through_regress/models.py +++ b/tests/regressiontests/m2m_through_regress/models.py @@ -8,7 +8,7 @@ class Membership(models.Model): person = models.ForeignKey('Person') group = models.ForeignKey('Group') price = models.IntegerField(default=100) - + def __unicode__(self): return "%s is a member of %s" % (self.person.name, self.group.name) @@ -16,7 +16,7 @@ class UserMembership(models.Model): user = models.ForeignKey(User) group = models.ForeignKey('Group') price = models.IntegerField(default=100) - + def __unicode__(self): return "%s is a user and member of %s" % (self.user.username, self.group.name) @@ -31,10 +31,10 @@ class Group(models.Model): # Membership object defined as a class members = models.ManyToManyField(Person, through=Membership) user_members = models.ManyToManyField(User, through='UserMembership') - + def __unicode__(self): return self.name - + __test__ = {'API_TESTS':""" # Create some dummy data >>> bob = Person.objects.create(name='Bob') @@ -46,7 +46,7 @@ __test__ = {'API_TESTS':""" >>> frank = User.objects.create_user('frank','frank@example.com','password') >>> jane = User.objects.create_user('jane','jane@example.com','password') -# Now test that the forward declared Membership works +# Now test that the forward declared Membership works >>> Membership.objects.create(person=bob, group=rock) @@ -83,7 +83,7 @@ Traceback (most recent call last): ... AttributeError: Cannot use create() on a ManyToManyField which specifies an intermediary model. Use Membership's Manager instead. -# Now test that the intermediate with a relationship outside +# Now test that the intermediate with a relationship outside # the current app (i.e., UserMembership) workds >>> UserMembership.objects.create(user=frank, group=rock) @@ -100,11 +100,11 @@ AttributeError: Cannot use create() on a ManyToManyField which specifies an inte >>> roll.user_members.all() [] -# Regression test for #8134 -- +# Regression test for #8134 -- # m2m-through models shouldn't be serialized as m2m fields on the model. -# First, clean up a lot of objects we don't need. -# The serialization test only requires three objects to work - +# First, clean up a lot of objects we don't need. +# The serialization test only requires three objects to work - # one for each end of the m2m, plus the through model. >>> User.objects.all().delete() @@ -117,24 +117,24 @@ AttributeError: Cannot use create() on a ManyToManyField which specifies an inte >>> management.call_command('dumpdata', 'm2m_through_regress', format='json', indent=2) [ { - "pk": 2, - "model": "m2m_through_regress.membership", + "pk": 2, + "model": "m2m_through_regress.membership", "fields": { - "person": 1, - "price": 100, + "person": 1, + "price": 100, "group": 2 } - }, + }, { - "pk": 1, - "model": "m2m_through_regress.person", + "pk": 1, + "model": "m2m_through_regress.person", "fields": { "name": "Bob" } - }, + }, { - "pk": 2, - "model": "m2m_through_regress.group", + "pk": 2, + "model": "m2m_through_regress.group", "fields": { "name": "Roll" } @@ -158,4 +158,18 @@ AttributeError: Cannot use create() on a ManyToManyField which specifies an inte -"""} \ No newline at end of file +## Regression test for #8046: +Check that we don't involve too many copies of the intermediate table when +doing a join. + +>>> bob = Person.objects.create(name='Bob') +>>> jim = Person.objects.create(name='Jim') +>>> rock = Group.objects.create(name='Rock') +>>> roll = Group.objects.create(name='Roll') +>>> _ = Membership.objects.create(person=bob, group=rock) +>>> _ = Membership.objects.create(person=jim, group=rock, price=50) +>>> _ = Membership.objects.create(person=bob, group=roll, price=50) +>>> rock.members.filter(membership__price=50) +[] + +"""}