[2.2.x] Fixed #30385 -- Restored SearchVector(config) immutability.

Regression in 1a28dc3887.

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 405c836336 from master
This commit is contained in:
Simon Charette 2019-04-19 02:39:25 -04:00 committed by Mariusz Felisiak
parent 47885278c6
commit 88bf635c35
3 changed files with 34 additions and 4 deletions

View File

@ -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.expressions import CombinedExpression, Func, Value
from django.db.models.functions import Cast, Coalesce
from django.db.models.lookups import Lookup from django.db.models.lookups import Lookup
@ -45,8 +46,7 @@ class SearchVectorCombinable:
class SearchVector(SearchVectorCombinable, Func): class SearchVector(SearchVectorCombinable, Func):
function = 'to_tsvector' function = 'to_tsvector'
arg_joiner = ", ' '," arg_joiner = " || ' ' || "
template = '%(function)s(concat(%(expressions)s))'
output_field = SearchVectorField() output_field = SearchVectorField()
config = None 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): 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 = 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 self.config:
if not hasattr(self.config, 'resolve_expression'): if not hasattr(self.config, 'resolve_expression'):
resolved.config = Value(self.config).resolve_expression(query, allow_joins, reuse, summarize, for_save) 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 template is None:
if self.config: if self.config:
config_sql, config_params = compiler.compile(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: else:
template = self.template template = self.template
sql, params = super().as_sql(compiler, connection, function=function, template=template) sql, params = super().as_sql(compiler, connection, function=function, template=template)

View File

@ -42,3 +42,7 @@ Bugfixes
* Fixed a regression in Django 2.2 where ``IntegerField`` validation of * Fixed a regression in Django 2.2 where ``IntegerField`` validation of
database limits crashes if ``limit_value`` attribute in a custom validator is database limits crashes if ``limit_value`` attribute in a custom validator is
callable (:ticket:`30328`). 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`).

View File

@ -8,6 +8,7 @@ transcript.
from django.contrib.postgres.search import ( from django.contrib.postgres.search import (
SearchQuery, SearchRank, SearchVector, SearchQuery, SearchRank, SearchVector,
) )
from django.db import connection
from django.db.models import F from django.db.models import F
from django.test import SimpleTestCase, modify_settings, skipUnlessDBFeature from django.test import SimpleTestCase, modify_settings, skipUnlessDBFeature
@ -346,6 +347,23 @@ class TestRankingAndWeights(GrailTestData, PostgreSQLTestCase):
self.assertSequenceEqual(searched, [self.verse0]) 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): class SearchQueryTests(SimpleTestCase):
def test_str(self): def test_str(self):
tests = ( tests = (