From 3fe3887a2ed94f7b15be769f6d81571031ec5627 Mon Sep 17 00:00:00 2001 From: Adam Chainz Date: Thu, 10 Sep 2015 17:07:09 +0100 Subject: [PATCH] Fixed #25377 -- Changed Count queries to execute COUNT(*) instead of COUNT('*'). --- django/db/models/aggregates.py | 4 ++-- django/db/models/expressions.py | 8 ++++++++ docs/releases/1.8.5.txt | 4 ++++ tests/aggregation/tests.py | 6 ++++++ tests/expressions/tests.py | 1 + tests/test_runner/test_debug_sql.py | 24 ++++++++++++------------ 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/django/db/models/aggregates.py b/django/db/models/aggregates.py index 9e1a0b370b..dfae09e34f 100644 --- a/django/db/models/aggregates.py +++ b/django/db/models/aggregates.py @@ -2,7 +2,7 @@ Classes to represent the definitions of aggregate functions. """ from django.core.exceptions import FieldError -from django.db.models.expressions import Func, Value +from django.db.models.expressions import Func, Star from django.db.models.fields import FloatField, IntegerField __all__ = [ @@ -98,7 +98,7 @@ class Count(Aggregate): def __init__(self, expression, distinct=False, **extra): if expression == '*': - expression = Value(expression) + expression = Star() super(Count, self).__init__( expression, distinct='DISTINCT ' if distinct else '', output_field=IntegerField(), **extra) diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 3ace285704..0271c7d3d3 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -593,6 +593,14 @@ class RawSQL(Expression): return [self] +class Star(Expression): + def __repr__(self): + return "'*'" + + def as_sql(self, compiler, connection): + return '*', [] + + class Random(Expression): def __init__(self): super(Random, self).__init__(output_field=fields.FloatField()) diff --git a/docs/releases/1.8.5.txt b/docs/releases/1.8.5.txt index 917599447f..1141c2c798 100644 --- a/docs/releases/1.8.5.txt +++ b/docs/releases/1.8.5.txt @@ -32,3 +32,7 @@ Bugfixes * Fixed migrations crash on MySQL when adding a text or a blob field with an unhashable default (:ticket:`25393`). + +* Changed ``Count`` queries to execute ``COUNT(*)`` instead of ``COUNT('*')`` + as versions of Django before 1.8 did (:ticket:`25377`). This may fix a + performance regression on some databases. diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 1472bd04fd..a1d12e363e 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -405,6 +405,12 @@ class AggregateTestCase(TestCase): vals = Book.objects.aggregate(Count("rating", distinct=True)) self.assertEqual(vals, {"rating__count": 4}) + def test_count_star(self): + with self.assertNumQueries(1) as ctx: + Book.objects.aggregate(n=Count("*")) + sql = ctx.captured_queries[0]['sql'] + self.assertIn('SELECT COUNT(*) ', sql) + def test_non_grouped_annotation_not_in_group_by(self): """ An annotation not included in values() before an aggregate should be diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index 013498a44d..a18084e06f 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -878,6 +878,7 @@ class ReprTests(TestCase): def test_aggregates(self): self.assertEqual(repr(Avg('a')), "Avg(F(a))") self.assertEqual(repr(Count('a')), "Count(F(a), distinct=False)") + self.assertEqual(repr(Count('*')), "Count('*', distinct=False)") self.assertEqual(repr(Max('a')), "Max(F(a))") self.assertEqual(repr(Min('a')), "Min(F(a))") self.assertEqual(repr(StdDev('a')), "StdDev(F(a), sample=False)") diff --git a/tests/test_runner/test_debug_sql.py b/tests/test_runner/test_debug_sql.py index fa7c7f7401..b943e83e31 100644 --- a/tests/test_runner/test_debug_sql.py +++ b/tests/test_runner/test_debug_sql.py @@ -63,25 +63,25 @@ class TestDebugSQL(unittest.TestCase): if six.PY3: expected_outputs = [ - ('''QUERY = 'SELECT COUNT(%s) AS "__count" ''' + ('''QUERY = 'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = ('*', 'error');'''), - ('''QUERY = 'SELECT COUNT(%s) AS "__count" ''' + '''- PARAMS = ('error',);'''), + ('''QUERY = 'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = ('*', 'fail');'''), + '''- PARAMS = ('fail',);'''), ] else: expected_outputs = [ - ('''QUERY = u'SELECT COUNT(%s) AS "__count" ''' + ('''QUERY = u'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = (u'*', u'error');'''), - ('''QUERY = u'SELECT COUNT(%s) AS "__count" ''' + '''- PARAMS = (u'error',);'''), + ('''QUERY = u'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = (u'*', u'fail');'''), + '''- PARAMS = (u'fail',);'''), ] verbose_expected_outputs = [ @@ -94,15 +94,15 @@ class TestDebugSQL(unittest.TestCase): ] if six.PY3: verbose_expected_outputs += [ - ('''QUERY = 'SELECT COUNT(%s) AS "__count" ''' + ('''QUERY = 'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = ('*', 'pass');'''), + '''- PARAMS = ('pass',);'''), ] else: verbose_expected_outputs += [ - ('''QUERY = u'SELECT COUNT(%s) AS "__count" ''' + ('''QUERY = u'SELECT COUNT(*) AS "__count" ''' '''FROM "test_runner_person" WHERE ''' '''"test_runner_person"."first_name" = %s' ''' - '''- PARAMS = (u'*', u'pass');'''), + '''- PARAMS = (u'pass',);'''), ]