Opened 4 years ago

Closed 4 years ago

Last modified 4 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: master
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 Changed 4 years ago by Simon Charette

Needs tests: set
Triage Stage: UnreviewedAccepted
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 Changed 4 years ago by Jakub Roztočil

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 Changed 4 years ago by Jakub Roztočil

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

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

Resolution: fixed
Status: newclosed

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 Changed 4 years ago by Anssi Kääriäinen <akaariai@…>

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