Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#22489 closed Bug (fixed)

Missing as_sql() implementation for __search lookup

Reported by: Jakub Roztočil Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: mysql, search, full-text, __search, lookup
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Change History (5)

comment:1 by Simon Charette, 10 years ago

Needs tests: set
Triage Stage: Unreviewed → Accepted
Version: → master

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.

comment:2 by Jakub Roztočil, 10 years ago

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)

comment:3 by Jakub Roztočil, 10 years ago

Is there anything else missing for the ​pull request to be merged?

comment:4 by Anssi Kääriäinen <akaariai@…>, 10 years ago

Resolution: → fixed
Status: new → closed

In 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.

comment:5 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In 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

Note: See TracTickets for help on using tickets.
Back to Top