Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #8046 -- The first filter() call on a related manager for many-…

…to-many

fields no longer creates duplicate copies of the join table(s). Basically, this
means filters on the join table (for ManyToManyField(through=...)) and complex
filters in the normal (non-through) case don't produce incorrect or duplicate
results.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@8472 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit d4d7bc175ddd0b698e394a202f8ab6ae572d63d3 1 parent 6e36ce1
@malcolmt malcolmt authored
View
2  django/db/models/fields/related.py
@@ -376,7 +376,7 @@ def __init__(self, model=None, core_filters=None, instance=None, symmetrical=Non
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.
View
17 django/db/models/query.py
@@ -121,6 +121,7 @@ def __init__(self, model=None, query=None):
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 @@ def reverse(self):
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 @@ def _fill_cache(self, num=None):
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
View
50 django/db/models/sql/query.py
@@ -58,6 +58,8 @@ def __init__(self, model, connection, where=WhereNode):
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 @@ def __init__(self, model, connection, where=WhereNode):
# 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 @@ def clone(self, klass=None, **kwargs):
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 @@ def add_q(self, q_object, used_aliases=None):
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):
View
54 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 @@ def __unicode__(self):
>>> 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)
<Membership: Bob is a member of Rock>
@@ -83,7 +83,7 @@ def __unicode__(self):
...
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)
<UserMembership: frank is a user and member of Rock>
@@ -100,11 +100,11 @@ def __unicode__(self):
>>> roll.user_members.all()
[<User: frank>]
-# 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 @@ def __unicode__(self):
>>> 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 @@ def __unicode__(self):
</object>
</django-objects>
-"""}
+## 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)
+[<Person: Jim>]
+
+"""}
Please sign in to comment.
Something went wrong with that request. Please try again.