Fixed #11535: Corrected SQL generated for queries involving generic r…

…elations and ORed Q objects. Thanks to brianglass for report, tobias for fix and test, and Alex for review.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12405 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/db/models/sql/query.py

@@ -1100,13 +1100,13 @@ def add_q(self, q_object, used_aliases=None):
             for child in q_object.children:
                 if connector == OR:
                     refcounts_before = self.alias_refcount.copy()
+                self.where.start_subtree(connector)
                 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)
+                self.where.end_subtree()
                 if connector == OR:
                     # Aliases that were newly added or not used at all need to
                     # be promoted to outer joins if they are nullable relations.

tests/regressiontests/generic_relations_regress/models.py

@@ -2,6 +2,10 @@
 from django.contrib.contenttypes import generic
 from django.contrib.contenttypes.models import ContentType
 
+__all__ = ('Link', 'Place', 'Restaurant', 'Person', 'Address', 
+           'CharLink', 'TextLink', 'OddRelation1', 'OddRelation2', 
+           'Contact', 'Organization', 'Note')
+
 class Link(models.Model):
     content_type = models.ForeignKey(ContentType)
     object_id = models.PositiveIntegerField()
@@ -59,3 +63,17 @@ class OddRelation2(models.Model):
     name = models.CharField(max_length=100)
     tlinks = generic.GenericRelation(TextLink)
 
+# models for test_q_object_or:
+class Note(models.Model):
+    content_type = models.ForeignKey(ContentType)
+    object_id = models.PositiveIntegerField()
+    content_object = generic.GenericForeignKey()
+    note = models.TextField()
+
+class Contact(models.Model):
+    notes = generic.GenericRelation(Note)
+
+class Organization(models.Model):
+    name = models.CharField(max_length=255)
+    contacts = models.ManyToManyField(Contact, related_name='organizations')
+

tests/regressiontests/generic_relations_regress/tests.py

@@ -1,6 +1,7 @@
 from django.test import TestCase
 from django.contrib.contenttypes.models import ContentType
-from models import Link, Place, Restaurant, Person, Address, CharLink, TextLink, OddRelation1, OddRelation2
+from django.db.models import Q
+from models import *
 
 class GenericRelationTests(TestCase):
 
@@ -40,3 +41,34 @@ def test_textlink_delete(self):
         oddrel = OddRelation2.objects.create(name='tlink')
         tl = TextLink.objects.create(content_object=oddrel)
         oddrel.delete()
+
+    def test_q_object_or(self):
+        """
+        Tests that SQL query parameters for generic relations are properly
+        grouped when OR is used.
+
+        Test for bug http://code.djangoproject.com/ticket/11535
+
+        In this bug the first query (below) works while the second, with the
+        query parameters the same but in reverse order, does not.
+
+        The issue is that the generic relation conditions do not get properly
+        grouped in parentheses.
+        """
+        note_contact = Contact.objects.create()
+        org_contact = Contact.objects.create()
+        note = Note.objects.create(note='note', content_object=note_contact)
+        org = Organization.objects.create(name='org name')
+        org.contacts.add(org_contact)
+        # search with a non-matching note and a matching org name
+        qs = Contact.objects.filter(Q(notes__note__icontains=r'other note') |
+                            Q(organizations__name__icontains=r'org name'))
+        self.assertTrue(org_contact in qs)
+        # search again, with the same query parameters, in reverse order
+        qs = Contact.objects.filter(
+            Q(organizations__name__icontains=r'org name') |
+            Q(notes__note__icontains=r'other note'))
+        self.assertTrue(org_contact in qs)
+
+
+