From 88bf635c356b4d3a47e88dc4142b90060ce3c2ef Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 19 Apr 2019 02:39:25 -0400 Subject: [PATCH] [2.2.x] Fixed #30385 -- Restored SearchVector(config) immutability. Regression in 1a28dc3887e8d66d5e3ff08cf7fb0a6212b873e5. The usage of CONCAT to allow SearchVector to deal with non-text fields made the generated expression non-IMMUTABLE which prevents a functional index to be created for it. Using a combination of COALESCE and ::text makes sure the expression preserves its immutability. Refs #29582. Thanks Andrew Brown for the report, Nick Pope for the review. Backport of 405c8363362063542e9e79beac53c8437d389520 from master --- django/contrib/postgres/search.py | 16 ++++++++++++---- docs/releases/2.2.1.txt | 4 ++++ tests/postgres_tests/test_search.py | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/django/contrib/postgres/search.py b/django/contrib/postgres/search.py index 9cd79623b3..f8c691d73b 100644 --- a/django/contrib/postgres/search.py +++ b/django/contrib/postgres/search.py @@ -1,5 +1,6 @@ -from django.db.models import Field, FloatField +from django.db.models import CharField, Field, FloatField, TextField from django.db.models.expressions import CombinedExpression, Func, Value +from django.db.models.functions import Cast, Coalesce from django.db.models.lookups import Lookup @@ -45,8 +46,7 @@ class SearchVectorCombinable: class SearchVector(SearchVectorCombinable, Func): function = 'to_tsvector' - arg_joiner = ", ' '," - template = '%(function)s(concat(%(expressions)s))' + arg_joiner = " || ' ' || " output_field = SearchVectorField() config = None @@ -60,6 +60,14 @@ class SearchVector(SearchVectorCombinable, Func): def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False): resolved = super().resolve_expression(query, allow_joins, reuse, summarize, for_save) + resolved.set_source_expressions([ + Coalesce( + expression + if isinstance(expression.output_field, (CharField, TextField)) + else Cast(expression, TextField()), + Value('') + ) for expression in resolved.get_source_expressions() + ]) if self.config: if not hasattr(self.config, 'resolve_expression'): resolved.config = Value(self.config).resolve_expression(query, allow_joins, reuse, summarize, for_save) @@ -72,7 +80,7 @@ class SearchVector(SearchVectorCombinable, Func): if template is None: if self.config: config_sql, config_params = compiler.compile(self.config) - template = "%(function)s({}::regconfig, concat(%(expressions)s))".format(config_sql.replace('%', '%%')) + template = '%(function)s({}::regconfig, %(expressions)s)'.format(config_sql.replace('%', '%%')) else: template = self.template sql, params = super().as_sql(compiler, connection, function=function, template=template) diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index 97fc9fafcd..48cbfb3a8f 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -42,3 +42,7 @@ Bugfixes * Fixed a regression in Django 2.2 where ``IntegerField`` validation of database limits crashes if ``limit_value`` attribute in a custom validator is callable (:ticket:`30328`). + +* Fixed a regression in Django 2.2 where + :class:`~django.contrib.postgres.search.SearchVector` generates SQL that is + not indexable (:ticket:`30385`). diff --git a/tests/postgres_tests/test_search.py b/tests/postgres_tests/test_search.py index 303f4d8783..8944c6342d 100644 --- a/tests/postgres_tests/test_search.py +++ b/tests/postgres_tests/test_search.py @@ -8,6 +8,7 @@ transcript. from django.contrib.postgres.search import ( SearchQuery, SearchRank, SearchVector, ) +from django.db import connection from django.db.models import F from django.test import SimpleTestCase, modify_settings, skipUnlessDBFeature @@ -346,6 +347,23 @@ class TestRankingAndWeights(GrailTestData, PostgreSQLTestCase): self.assertSequenceEqual(searched, [self.verse0]) +class SearchVectorIndexTests(PostgreSQLTestCase): + def test_search_vector_index(self): + """SearchVector generates IMMUTABLE SQL in order to be indexable.""" + # This test should be moved to test_indexes and use a functional + # index instead once support lands (see #26167). + query = Line.objects.all().query + resolved = SearchVector('id', 'dialogue', config='english').resolve_expression(query) + compiler = query.get_compiler(connection.alias) + sql, params = resolved.as_sql(compiler, connection) + # Indexed function must be IMMUTABLE. + with connection.cursor() as cursor: + cursor.execute( + 'CREATE INDEX search_vector_index ON %s USING GIN (%s)' % (Line._meta.db_table, sql), + params, + ) + + class SearchQueryTests(SimpleTestCase): def test_str(self): tests = (