From 42e029f6c4d9b03a238c3f8b479258ca2cb034ed Mon Sep 17 00:00:00 2001 From: Josh Smeaton Date: Wed, 14 Oct 2015 10:07:42 +1100 Subject: [PATCH] [1.8.x] Fixed #25517 -- Made Concat function idempotent on SQLite. Backport of 6c95b134e9b2d5641c123551c080305e90e6a89d from master --- django/db/models/functions.py | 15 +++++++++------ docs/releases/1.8.6.txt | 2 ++ tests/db_functions/tests.py | 18 +++++++++++++++++- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/django/db/models/functions.py b/django/db/models/functions.py index 610ecb6985..531f9a882f 100644 --- a/django/db/models/functions.py +++ b/django/db/models/functions.py @@ -41,10 +41,10 @@ class ConcatPair(Func): super(ConcatPair, self).__init__(left, right, **extra) def as_sqlite(self, compiler, connection): - self.arg_joiner = ' || ' - self.template = '%(expressions)s' - self.coalesce() - return super(ConcatPair, self).as_sql(compiler, connection) + coalesced = self.coalesce() + coalesced.arg_joiner = ' || ' + coalesced.template = '%(expressions)s' + return super(ConcatPair, coalesced).as_sql(compiler, connection) def as_mysql(self, compiler, connection): self.coalesce() @@ -52,9 +52,12 @@ class ConcatPair(Func): def coalesce(self): # null on either side results in null for expression, wrap with coalesce + c = self.copy() expressions = [ - Coalesce(expression, Value('')) for expression in self.get_source_expressions()] - self.set_source_expressions(expressions) + Coalesce(expression, Value('')) for expression in c.get_source_expressions() + ] + c.set_source_expressions(expressions) + return c class Concat(Func): diff --git a/docs/releases/1.8.6.txt b/docs/releases/1.8.6.txt index 44380cded5..178d714f01 100644 --- a/docs/releases/1.8.6.txt +++ b/docs/releases/1.8.6.txt @@ -23,3 +23,5 @@ Bugfixes have their reverse relations disabled (:ticket:`25545`). * Allowed filtering over a ``RawSQL`` annotation (:ticket:`25506`). + +* Made the ``Concat`` database function idempotent on SQLite (:ticket:`25517`). diff --git a/tests/db_functions/tests.py b/tests/db_functions/tests.py index f07e8770f0..5fd3b74fbd 100644 --- a/tests/db_functions/tests.py +++ b/tests/db_functions/tests.py @@ -1,8 +1,11 @@ from __future__ import unicode_literals +from unittest import skipUnless + +from django.db import connection from django.db.models import CharField, TextField, Value as V from django.db.models.functions import ( - Coalesce, Concat, Length, Lower, Substr, Upper, + Coalesce, Concat, ConcatPair, Length, Lower, Substr, Upper, ) from django.test import TestCase from django.utils import six, timezone @@ -154,6 +157,19 @@ class FunctionTests(TestCase): expected = article.title + ' - ' + article.text self.assertEqual(expected.upper(), article.title_text) + @skipUnless(connection.vendor == 'sqlite', "sqlite specific implementation detail.") + def test_concat_coalesce_idempotent(self): + pair = ConcatPair(V('a'), V('b')) + # Check nodes counts + self.assertEqual(len(list(pair.flatten())), 3) + self.assertEqual(len(list(pair.coalesce().flatten())), 7) # + 2 Coalesce + 2 Value() + self.assertEqual(len(list(pair.flatten())), 3) + + def test_concat_sql_generation_idempotency(self): + qs = Article.objects.annotate(description=Concat('title', V(': '), 'summary')) + # Multiple compilations should not alter the generated query. + self.assertEqual(str(qs.query), str(qs.all().query)) + def test_lower(self): Author.objects.create(name='John Smith', alias='smithj') Author.objects.create(name='Rhonda')