Added support for modifying the effect of ``DISTINCT`` clauses so they

only consider some fields (PostgreSQL only).

For this, the ``distinct()`` QuerySet method now accepts an optional
list of model fields names and generates ``DISTINCT ON`` clauses on
these cases. Thanks Jeffrey Gelens and Anssi Kääriäinen for their work.

Fixes #6422.

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

6 files changed, 175 insertions, 11 deletions

diff --git a/tests/modeltests/distinct_on_fields/__init__.py b/tests/modeltests/distinct_on_fields/__init__.py
new file mode 100644
index 0000000000..792d600548
--- /dev/null
+++ b/tests/modeltests/distinct_on_fields/__init__.py
@@ -0,0 +1 @@
+#
diff --git a/tests/modeltests/distinct_on_fields/models.py b/tests/modeltests/distinct_on_fields/models.py
new file mode 100644
index 0000000000..be0b591107
--- /dev/null
+++ b/tests/modeltests/distinct_on_fields/models.py
@@ -0,0 +1,39 @@
+from django.db import models
+
+class Tag(models.Model):
+    name = models.CharField(max_length=10)
+    parent = models.ForeignKey('self', blank=True, null=True,
+            related_name='children')
+
+    class Meta:
+        ordering = ['name']
+
+    def __unicode__(self):
+        return self.name
+
+class Celebrity(models.Model):
+    name = models.CharField("Name", max_length=20)
+    greatest_fan = models.ForeignKey("Fan", null=True, unique=True)
+
+    def __unicode__(self):
+        return self.name
+
+class Fan(models.Model):
+    fan_of = models.ForeignKey(Celebrity)
+
+class Staff(models.Model):
+    id = models.IntegerField(primary_key=True)
+    name = models.CharField(max_length=50)
+    organisation = models.CharField(max_length=100)
+    tags = models.ManyToManyField(Tag, through='StaffTag')
+    coworkers = models.ManyToManyField('self')
+
+    def __unicode__(self):
+        return self.name
+
+class StaffTag(models.Model):
+    staff = models.ForeignKey(Staff)
+    tag = models.ForeignKey(Tag)
+
+    def __unicode__(self):
+        return u"%s -> %s" % (self.tag, self.staff)
diff --git a/tests/modeltests/distinct_on_fields/tests.py b/tests/modeltests/distinct_on_fields/tests.py
new file mode 100644
index 0000000000..5021bc8088
--- /dev/null
+++ b/tests/modeltests/distinct_on_fields/tests.py
@@ -0,0 +1,116 @@
+from __future__ import absolute_import, with_statement
+
+from django.db.models import Max
+from django.test import TestCase, skipUnlessDBFeature
+
+from .models import Tag, Celebrity, Fan, Staff, StaffTag
+
+class DistinctOnTests(TestCase):
+    def setUp(self):
+        t1 = Tag.objects.create(name='t1')
+        t2 = Tag.objects.create(name='t2', parent=t1)
+        t3 = Tag.objects.create(name='t3', parent=t1)
+        t4 = Tag.objects.create(name='t4', parent=t3)
+        t5 = Tag.objects.create(name='t5', parent=t3)
+
+        p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1")
+        p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1")
+        p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1")
+        p1_o2 = Staff.objects.create(id=4, name="p1", organisation="o2")
+        p1_o1.coworkers.add(p2_o1, p3_o1)
+        StaffTag.objects.create(staff=p1_o1, tag=t1)
+        StaffTag.objects.create(staff=p1_o1, tag=t1)
+
+        celeb1 = Celebrity.objects.create(name="c1")
+        celeb2 = Celebrity.objects.create(name="c2")
+
+        self.fan1 = Fan.objects.create(fan_of=celeb1)
+        self.fan2 = Fan.objects.create(fan_of=celeb1)
+        self.fan3 = Fan.objects.create(fan_of=celeb2)
+
+    @skipUnlessDBFeature('can_distinct_on_fields')
+    def test_basic_distinct_on(self):
+        """QuerySet.distinct('field', ...) works"""
+        # (qset, expected) tuples
+        qsets = (
+            (
+                Staff.objects.distinct().order_by('name'),
+                ['<Staff: p1>', '<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Staff.objects.distinct('name').order_by('name'),
+                ['<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Staff.objects.distinct('organisation').order_by('organisation', 'name'),
+                ['<Staff: p1>', '<Staff: p1>'],
+            ),
+            (
+                Staff.objects.distinct('name', 'organisation').order_by('name', 'organisation'),
+                ['<Staff: p1>', '<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Celebrity.objects.filter(fan__in=[self.fan1, self.fan2, self.fan3]).\
+                    distinct('name').order_by('name'),
+                ['<Celebrity: c1>', '<Celebrity: c2>'],
+            ),
+            # Does combining querysets work?
+            (
+                (Celebrity.objects.filter(fan__in=[self.fan1, self.fan2]).\
+                    distinct('name').order_by('name')
+                |Celebrity.objects.filter(fan__in=[self.fan3]).\
+                    distinct('name').order_by('name')),
+                ['<Celebrity: c1>', '<Celebrity: c2>'],
+            ),
+            (
+                StaffTag.objects.distinct('staff','tag'),
+                ['<StaffTag: t1 -> p1>'],
+            ),
+            (
+                Tag.objects.order_by('parent__pk', 'pk').distinct('parent'),
+                ['<Tag: t2>', '<Tag: t4>', '<Tag: t1>'],
+            ),
+            (
+                StaffTag.objects.select_related('staff').distinct('staff__name').order_by('staff__name'),
+                ['<StaffTag: t1 -> p1>'],
+            ),
+            # Fetch the alphabetically first coworker for each worker
+            (
+                (Staff.objects.distinct('id').order_by('id', 'coworkers__name').
+                               values_list('id', 'coworkers__name')),
+                ["(1, u'p2')", "(2, u'p1')", "(3, u'p1')", "(4, None)"]
+            ),
+        )
+        for qset, expected in qsets:
+            self.assertQuerysetEqual(qset, expected)
+            self.assertEqual(qset.count(), len(expected))
+
+        # Combining queries with different distinct_fields is not allowed.
+        base_qs = Celebrity.objects.all()
+        self.assertRaisesMessage(
+            AssertionError,
+            "Cannot combine queries with different distinct fields.",
+            lambda: (base_qs.distinct('id') & base_qs.distinct('name'))
+        )
+
+        # Test join unreffing
+        c1 = Celebrity.objects.distinct('greatest_fan__id', 'greatest_fan__fan_of')
+        self.assertIn('OUTER JOIN', str(c1.query))
+        c2 = c1.distinct('pk')
+        self.assertNotIn('OUTER JOIN', str(c2.query))
+
+    @skipUnlessDBFeature('can_distinct_on_fields')
+    def test_distinct_not_implemented_checks(self):
+        # distinct + annotate not allowed
+        with self.assertRaises(NotImplementedError):
+            Celebrity.objects.annotate(Max('id')).distinct('id')[0]
+        with self.assertRaises(NotImplementedError):
+            Celebrity.objects.distinct('id').annotate(Max('id'))[0]
+
+        # However this check is done only when the query executes, so you
+        # can use distinct() to remove the fields before execution.
+        Celebrity.objects.distinct('id').annotate(Max('id')).distinct()[0]
+        # distinct + aggregate not allowed
+        with self.assertRaises(NotImplementedError):
+            Celebrity.objects.distinct('id').aggregate(Max('id'))
+
diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py
index e69ce48ab1..2f4c1453d7 100644
--- a/tests/regressiontests/queries/models.py
+++ b/tests/regressiontests/queries/models.py
@@ -209,6 +209,9 @@ class Celebrity(models.Model):
     name = models.CharField("Name", max_length=20)
     greatest_fan = models.ForeignKey("Fan", null=True, unique=True)
 
+    def __unicode__(self):
+        return self.name
+
 class TvChef(Celebrity):
     pass
 
@@ -343,4 +346,3 @@ class OneToOneCategory(models.Model):
 
     def __unicode__(self):
         return "one2one " + self.new_name
-
diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py
index f7181d17c9..8e9705e9f7 100644
--- a/tests/regressiontests/queries/tests.py
+++ b/tests/regressiontests/queries/tests.py
@@ -234,18 +234,22 @@ class Queries1Tests(BaseQuerysetTest):
             ['<Item: four>', '<Item: one>']
         )
 
-    # FIXME: This is difficult to fix and very much an edge case, so punt for
-    # now.  This is related to the order_by() tests for ticket #2253, but the
-    # old bug exhibited itself here (q2 was pulling too many tables into the
-    # combined query with the new ordering, but only because we have evaluated
-    # q2 already).
-    @unittest.expectedFailure
     def test_order_by_tables(self):
         q1 = Item.objects.order_by('name')
         q2 = Item.objects.filter(id=self.i1.id)
         list(q2)
         self.assertEqual(len((q1 & q2).order_by('name').query.tables), 1)
 
+    def test_order_by_join_unref(self):
+        """
+        This test is related to the above one, testing that there aren't
+        old JOINs in the query.
+        """
+        qs = Celebrity.objects.order_by('greatest_fan__fan_of')
+        self.assertIn('OUTER JOIN', str(qs.query))
+        qs = qs.order_by('id')
+        self.assertNotIn('OUTER JOIN', str(qs.query))
+
     def test_tickets_4088_4306(self):
         self.assertQuerysetEqual(
             Report.objects.filter(creator=1001),
@@ -1728,7 +1732,7 @@ class ToFieldTests(TestCase):
 
 
 class ConditionalTests(BaseQuerysetTest):
-    """Tests whose execution depend on dfferent environment conditions like
+    """Tests whose execution depend on different environment conditions like
     Python version or DB backend features"""
 
     def setUp(self):
@@ -1739,6 +1743,7 @@ class ConditionalTests(BaseQuerysetTest):
         t4 = Tag.objects.create(name='t4', parent=t3)
         t5 = Tag.objects.create(name='t5', parent=t3)
 
+
     # In Python 2.6 beta releases, exceptions raised in __len__ are swallowed
     # (Python issue 1242657), so these cases return an empty list, rather than
     # raising an exception. Not a lot we can do about that, unfortunately, due to
@@ -1810,6 +1815,7 @@ class ConditionalTests(BaseQuerysetTest):
             2500
         )
 
+
 class UnionTests(unittest.TestCase):
     """
     Tests for the union of two querysets. Bug #12252.
diff --git a/tests/regressiontests/select_related_regress/tests.py b/tests/regressiontests/select_related_regress/tests.py
index 4818b95cdd..4cd4f788e2 100644
--- a/tests/regressiontests/select_related_regress/tests.py
+++ b/tests/regressiontests/select_related_regress/tests.py
@@ -40,9 +40,9 @@ class SelectRelatedRegressTests(TestCase):
         self.assertEqual([(c.id, unicode(c.start), unicode(c.end)) for c in connections],
             [(c1.id, u'router/4', u'switch/7'), (c2.id, u'switch/7', u'server/1')])
 
-        # This final query should only join seven tables (port, device and building
-        # twice each, plus connection once).
-        self.assertEqual(connections.query.count_active_tables(), 7)
+        # This final query should only have seven tables (port, device and building
+        # twice each, plus connection once). Thus, 6 joins plus the FROM table.
+        self.assertEqual(str(connections.query).count(" JOIN "), 6)
 
 
     def test_regression_8106(self):