[Django] #30488: SearchVector lookup is generating redundent Coalesce wrapper
Django
------------------------------------------------+------------------------
Reporter: thomasina-lee | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.postgres | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
In Django 2.2.1, in the SearchVector if we do:
{{{
>>> from django.contrib.postgres.search import SearchVector
>>> Entry.objects.annotate(
... search=SearchVector('body_text', 'summary_text'),
... ).filter(search='Cheese')
}}}
The generated SQL currently looks something like
{{{
SELECT id /*......*/
FROM "app_entry" WHERE
to_tsvector(COALESCE(COALESCE("app_entry"."body_text", ''), '') || ' '
|| COALESCE(COALESCE("app_entry"."summary_text", ''), '')) @@
(plainto_tsquery('Cheese')) = true
}}}
i.e. in the {{{WHERE}}} clause, the fields is wrapped twice with
{{{COALESCE}}}, .e.g. {{{COALESCE(COALESCE("app_entry"."body_text", ),
)}}}
While it is still possible to create a functional index with this, the
generated SQL does not feel optimal at the very least.
It will be great if the generated SQL will keep to one level of
{{{COALESCE}}}, .e.g. {{{COALESCE("app_entry"."body_text", ), )}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30488>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: thomasina-lee | Owner: (none)
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* cc: Simon Charette (added)
* version: 2.2 => master
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. It looks that we resolve expression twice.
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: thomasina-lee | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: (none) => Simon Charette
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: T Lee | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by T Lee):
Sorry I didn't know I need to claim the ticket first. Anyhow here is a
pull request https://github.com/django/django/pull/11380 if it can be
used. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: T Lee | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
No worries, feel free to assign the ticket to you. The patch seems good
but you'll probably want to add an addition regression test which might
require building a queryset and asserting against
`self.assertNotIn('COALESCE(COALESCE', str(qs.query))`. You'll want to add
a mention in the 2.2.2 release notes as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: T Lee | Owner: T Lee
Type: Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by T Lee):
* owner: Simon Charette => T Lee
Comment:
Thanks. I have claimed the ticket, and have added both regression test
and 2.2.2 release mention in [https://github.com/django/django/pull/11380
PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: T Lee | Owner: T Lee
Type: Cleanup/optimization | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: T Lee | Owner: T Lee
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* version: master => 2.2
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: T Lee | Owner: T Lee
Component: contrib.postgres | Version: 2.2
Severity: Normal | Resolution: fixed
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 Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c38e7a79f4354ee831f92deb7a658fc0387e3bec" c38e7a79]:
{{{
#!CommitTicketReference repository=""
revision="c38e7a79f4354ee831f92deb7a658fc0387e3bec"
Fixed #30488 -- Removed redundant Coalesce call in SQL generated by
SearchVector.
Regression in 405c8363362063542e9e79beac53c8437d389520.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: T Lee | Owner: T Lee
Type: Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
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:"3d4e53bcb1f94e451f291e832bfa668361cd64a2" 3d4e53bc]:
{{{
#!CommitTicketReference repository=""
revision="3d4e53bcb1f94e451f291e832bfa668361cd64a2"
[2.2.x] Fixed #30488 -- Removed redundant Coalesce call in SQL generated
by SearchVector.
Regression in 405c8363362063542e9e79beac53c8437d389520.
Backport of c38e7a79f4354ee831f92deb7a658fc0387e3bec from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30488#comment:9>