[Django] #31340: Improve expression support for __search lookup and SearchQuery
Django
---------------------------------------------+------------------------
Reporter: Baptiste Mispelon | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
''(not sure whether to categorize this as a bug or a new feature)''
I've been trying to implement some kind of reverse full text search where
I store keywords in the database and I want to query models whose keywords
would match a given piece of text.
Here's a simplified model of what I'm working with:
{{{#!python
class SavedSearch(models.Model):
keywords = models.TextField()
def __str__(self):
return self.keywords
}}}
I've managed to achieve what I want in the case of the default search
configuration using annotation and wrapping things with `Value` or `Cast`:
{{{#!python
# This works
search_query = Cast('keywords', output_field=SearchQueryField())
search_vector = SearchVector(Value("lorem ipsum ...",
output_field=TextField()))
qs =
SavedSearch.objects.annotate(search=search_vector).filter(search=search_query)
}}}
But if I want to use a custom search configuration, things don't work
anymore:
{{{#!python
# This doesn't work (can't adapt type 'F')
search_query = SearchQuery(F('keywords'), config='english',
search_type='plain')
search_vector = SearchVector(Value("lorem ipsum ...",
output_field=TextField()))
qs =
SavedSearch.objects.annotate(search=search_vector).filter(search=search_query)
}}}
I'm not very familiar with the inner workings of `Lookup` objects but I
did some digging and I think I came up with a fix which involved fixing
two separate issues:
1) `SearchQuery` doesn't currently support anyting other than plain values
(`str`). Fixing this required changing both `resolve_expression()` and
`as_sql()`.
2) the `__search` lookup doesn't support things like `F` objects because
of it assumes that any value with a `resolve_expression` method must be a
`SearchQuery` object.
--
Ticket URL: <https://code.djangoproject.com/ticket/31340>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Baptiste Mispelon | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: 3.0
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Baptiste Mispelon):
Pull request here: https://github.com/django/django/pull/12525
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:1>
Django
Reporter: Baptiste Mispelon | Owner: (none)
Type: New feature | Status: new
Component: contrib.postgres | Version: 3.0
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:2>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: assigned
Component: contrib.postgres | Version: 3.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* owner: (none) => Baptiste Mispelon
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:3>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"dd704c670543edc4672cc6114fe3d8da0dfbed7a" dd704c6]:
{{{
#!CommitTicketReference repository=""
revision="dd704c670543edc4672cc6114fe3d8da0dfbed7a"
Refs #31340 -- Simplified SearchQuery by making it subclass Func.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:4>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* version: 3.0 => master
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:5>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3baf92cf8230ad3a932986170fd07c8feae7ff2f" 3baf92cf]:
{{{
#!CommitTicketReference repository=""
revision="3baf92cf8230ad3a932986170fd07c8feae7ff2f"
Fixed #31340 -- Allowed query expressions in SearchQuery.value and
__search lookup.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:6>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David):
Does the current implementation works well with custom SQL functions which
returns {{{tsquery}}} and does not use any field which is already a
{{{tsquery}}}?
In one of my project I have created a custom SQL function which takes a
list of integers (the IDs of some row in a specific table) and builds a
{{{tsquery}}}. I tought that using the following it would work well while
it raises an exception
{{{#!python
class TheModel(models.Model):
searchable = SearchVectorField()
TheModel.objects.filter(searchable=models.Func(123, 345,
function='my_tsquery_func', output_field=SearchQueryField())
# django.db.utils.ProgrammingError: function plainto_tsquery(tsquery) does
not exist
}}}
Because the SQL produced is and this produces an exception.
{{{#!sql
SELECT id, searchable
FROM myapp_themodel
WHERE searchable @@ plainto_tsquery(my_tsquery_func(123, 345));
}}}
To add more context the {{{my_tsquery_func}}} looks like the following
(the goal is to combine {{{tsvector}}}s into a {{{tsquery}}} to build a
similarity algorithm):
{{{#!sql
CREATE OR REPLACE FUNCTION my_tsquery_func(first_id bigint, second_id
bigint) RETURNS tsquery AS $$
SELECT
to_tsquery(
array_to_string(
tsvector_to_array(
(SELECT searchable FROM myapp_themodel WHERE id =
first_id LIMIT 1)
|| (SELECT searchable FROM myapp_themodel WHERE id =
second_id LIMIT 1)
),
' | '
)
);
$$ LANGUAGE sql PARALLEL SAFE STABLE;
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:7>
Django
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: closed
Component: contrib.postgres | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
I think the patch should have checked `output_field` instead of special
casing `SearchQuery` and `CombinedSearchQuery` in this form
{{{#!patch
diff --git a/django/contrib/postgres/search.py
b/django/contrib/postgres/search.py
index 2135c9bb88..636123a3ed 100644
--- a/django/contrib/postgres/search.py
+++ b/django/contrib/postgres/search.py
@@ -16,7 +16,10 @@ class SearchVectorExact(Lookup):
lookup_name = "exact"
def process_rhs(self, qn, connection):
- if not isinstance(self.rhs, (SearchQuery, CombinedSearchQuery)):
+ if not isinstance(
+ getattr(self.rhs, "_output_field_or_none", None),
+ SearchQueryField,
+ ):
config = getattr(self.lhs, "config", None)
self.rhs = SearchQuery(self.rhs, config=config)
rhs, rhs_params = super().process_rhs(qn, connection)
@@ -240,6 +243,15 @@ def __str__(self):
return "(%s)" % super().__str__()
+register_combinable_fields(
+ SearchQueryField, SearchQueryCombinable.BITAND, SearchQueryField,
SearchQueryField
+)
+
+register_combinable_fields(
+ SearchQueryField, SearchQueryCombinable.BITOR, SearchQueryField,
SearchQueryField
+)
+
+
class SearchRank(Func):
function = "ts_rank"
output_field = FloatField()
}}}
as in its current form it disallows usage of
`Func(output_field=SearchQueryField())`.
Happy to accept a new ticket with the above if you're willing to write
tests, in the mean time your best bet is likely to subclass `SearchQuery`
and override `__init__` to assign `my_tsquery_func` instead and support a
different signature.
--
Ticket URL: <https://code.djangoproject.com/ticket/31340#comment:8>