[Django] #22489: Missing as_sql() implementation for __search lookup

15 views
Skip to first unread message

Django

unread,
Apr 22, 2014, 12:42:00 PM4/22/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version:
(models, ORM) | Keywords: mysql, search, full-
Severity: Normal | text, __search, lookup
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
The (MySQL-only) __search full-text lookup isn't properly implemented
using the new ORM lookup system. It's use results to a runtime exception:


{{{
>>> MyModel.objects.filter(field__search='hello world')
[...]
return node.as_sql(self, self.connection)
File "/site-packages/django/db/models/sql/where.py", line 106, in as_sql
sql, params = qn.compile(child)
File "/site-packages/django/db/models/sql/compiler.py", line 80, in
compile
return node.as_sql(self, self.connection)
File "/site-packages/django/db/models/sql/where.py", line 106, in as_sql
sql, params = qn.compile(child)
File "/site-packages/django/db/models/sql/compiler.py", line 80, in
compile
return node.as_sql(self, self.connection)
File "/site-packages/django/db/models/lookups.py", line 150, in as_sql
rhs_sql = self.get_rhs_op(connection, rhs_sql)
File "/site-packages/django/db/models/lookups.py", line 154, in
get_rhs_op
return connection.operators[self.lookup_name] % rhs
KeyError: 'search'
}}}


I've opened a pull request here:

https://github.com/django/django/pull/2597

--
Ticket URL: <https://code.djangoproject.com/ticket/22489>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 22, 2014, 1:12:15 PM4/22/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: mysql, search, | Needs documentation: 0
full-text, __search, lookup | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):

* needs_better_patch: => 0
* version: => master
* needs_docs: => 0
* needs_tests: => 1
* stage: Unreviewed => Accepted


Comment:

Could you add a regression test for this?

Since it's only supported on MySQL atm you'll need to decorate your tests
with `@skipUnless(connection.vendor == 'mysql')`. Since Mark is working on
adding support for this to the postgresl backend we might want to add a
database feature flag and rely on `skipUnlessDbFeature` instead.

I guess those tests should live in `tests/lookup/tests.py`.

--
Ticket URL: <https://code.djangoproject.com/ticket/22489#comment:1>

Django

unread,
Apr 22, 2014, 4:15:12 PM4/22/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: mysql, search, | Needs documentation: 0
full-text, __search, lookup | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by jakub):

Added a test for MySQL.

It might be a good idea to add a DB feature flag, but the test still needs
MySQL-specific `CREATE FULLTEXT INDEX […]` however.

(I've accidentally messed up the original pull request and had to open a
new one: https://github.com/django/django/pull/2598)

--
Ticket URL: <https://code.djangoproject.com/ticket/22489#comment:2>

Django

unread,
Apr 30, 2014, 4:27:40 AM4/30/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: mysql, search, | Needs documentation: 0
full-text, __search, lookup | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by jakub):

Is there anything else missing for the
[https://github.com/django/django/pull/2598 pull request] to be merged?

--
Ticket URL: <https://code.djangoproject.com/ticket/22489#comment:3>

Django

unread,
May 5, 2014, 7:34:41 AM5/5/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: master
(models, ORM) | Resolution: fixed

Severity: Normal | Triage Stage: Accepted
Keywords: mysql, search, | Needs documentation: 0
full-text, __search, lookup | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by Anssi Kääriäinen <akaariai@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"7131e14d00b9b293cec4608925f2be6f94474350"]:
{{{
#!CommitTicketReference repository=""
revision="7131e14d00b9b293cec4608925f2be6f94474350"
Fixed #22489 -- missing implemenation for search lookup

When custom lookups were added, converting the search lookup to use
the new Lookup infrastructure wasn't done.

Some changes were needed to the added test, main change done by
committer was ensuring the test works on MySQL versions prior to 5.6.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22489#comment:4>

Django

unread,
May 5, 2014, 7:35:44 AM5/5/14
to django-...@googlegroups.com
#22489: Missing as_sql() implementation for __search lookup
-------------------------------------+-------------------------------------
Reporter: jakub | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Accepted
Keywords: mysql, search, | Needs documentation: 0
full-text, __search, lookup | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Anssi Kääriäinen <akaariai@…>):

In [changeset:"4b6ba2c1d1dcf6d7d587ee19b3565879ab8ee219"]:
{{{
#!CommitTicketReference repository=""
revision="4b6ba2c1d1dcf6d7d587ee19b3565879ab8ee219"
[1.7.x] Fixed #22489 -- missing implemenation for search lookup

When custom lookups were added, converting the search lookup to use
the new Lookup infrastructure wasn't done.

Some changes were needed to the added test, main change done by
committer was ensuring the test works on MySQL versions prior to 5.6.

Backport of 7131e14d00 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22489#comment:5>

Reply all
Reply to author
Forward
0 new messages