Opened 4 years ago

Closed 8 months ago

#16731 closed Bug (fixed)

startswith endswith and contains doesn't work with F expression

Reported by: ronnas@… Owned by: tchaumeny
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: startswith, F(), wildcards
Cc: kvsn Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by aaugustin)

The expression:

myTable.objects.filter(field1__startswith=F('field2'))

rendered to:

SELECT * FROM `my_table` WHERE `my_table`.`field1` LIKE `my_table`.`field2`

should be:

SELECT * FROM `my_table` WHERE `my_table`.`field1` LIKE CONCAT(`my_table`.`field2`,'%')

Attachments (1)

1117.patch.txt (2.5 KB) - added by mjtamlyn 2 years ago.
16731-failing-test.patch

Download all attachments as: .zip

Change History (14)

comment:1 Changed 4 years ago by aaugustin

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Fixed formatting - please use preview.

I have confirmed the problem as follows:

# todo/models.py

class Todo(models.Model):
    summary = models.CharField(max_length=100, verbose_name=_('summary'))
    details = models.TextField(blank=True, verbose_name=_('details'))
% ./manage.py shell

>>> from todo.models import *>>> from django.db.models import F
>>> Todo.objects.filter(details__startswith=F('summary'))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "django/db/models/query.py", line 66, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "django/db/models/query.py", line 81, in __len__
    self._result_cache.extend(self._iter)
  File "django/db/models/query.py", line 266, in iterator
    for row in compiler.results_iter():
  File "django/db/models/sql/compiler.py", line 699, in results_iter
    for rows in self.execute_sql(MULTI):
  File "django/db/models/sql/compiler.py", line 754, in execute_sql
    cursor.execute(sql, params)
  File "django/db/backends/util.py", line 34, in execute
    return self.cursor.execute(sql, params)
  File "django/db/backends/sqlite3/base.py", line 261, in execute
    return Database.Cursor.execute(self, query, params)
DatabaseError: near "ESCAPE": syntax error
>>> from django.db import connection
>>> connection.queries
[{'time': '0.000', 'sql': u'SELECT "todo_todo"."id", "todo_todo"."summary", "todo_todo"."details" FROM "todo_todo" WHERE "todo_todo"."details" LIKE  ESCAPE \'\\\' "todo_todo"."summary" LIMIT 21'}]

comment:2 Changed 4 years ago by anonymous

This issue also appears in "contains" query and subsequently in istartswith and icontains.

comment:3 Changed 2 years ago by kvsn

  • Cc kvsn added

I've added a test that should succeed when the ticket's been fixed: https://github.com/django/django/pull/1117. I've currently marked it as @expectedFailure.

Changed 2 years ago by mjtamlyn

16731-failing-test.patch

comment:4 Changed 2 years ago by mjtamlyn

  • Version 1.3 deleted

Thanks for your contribution. We can't merge this in without the fix, it's not quite the purpose of expectedFailure. I'll close the PR for now, I've added your failing test as a patch file to this ticket for posterity.

comment:5 Changed 2 years ago by uberj@…

This issue also happens when endswith is used in conjunction with an F expression.

comment:6 Changed 20 months ago by orontee

Since this bug was reported three years ago and is still there, I suggest to update the documentation (cf. https://docs.djangoproject.com/en/1.6/topics/db/queries/#filters-can-reference-fields-on-the-model) to say that F() expressions are broken with like queries.

Do you agree?

Last edited 20 months ago by orontee (previous) (diff)

comment:7 Changed 20 months ago by timo

In general we prefer to spend our time fixing bugs rather than documenting them. There are currently 575 open bugs in our ticket tracker. It would take a while to document them all. Hopefully this ticket serves as "documentation" in the sense that if someone searches for this problem they would hopefully find this ticket. That said, if someone were to look into fixing this and decides it's too complex to implement, then I would be open to calling this a limitation, documenting it, and closing the ticket.

comment:8 Changed 20 months ago by orontee

You are right.

For people reading this after they encounters that bug, a quick workaround is to replace:

myTable.filter(field1__startswith=F('field2'))

with:

myTable.extra(where=["""UPPER("my_table"."field1"::text) LIKE UPPER("my_table"."field2"||'%%')"""])
Last edited 20 months ago by orontee (previous) (diff)

comment:9 Changed 19 months ago by akaariai

The custom lookups branch has proof-of-concept implementation for this. See https://github.com/akaariai/django/commit/1016159f34674c0df871ed891cde72be8340bb5d.

The problem here is that the lookups are somewhat laborious to write correctly. For example the correct rhs operator for istartswith is something like:

    LIKE UPPER(replace(replace(%s, %%%%, '\\%%%%'), '_', '\\_')) || '%%%%')

That is, we need to replace % and _ chars in the field's value (%s is reference to the field in above) with escaped versions, then uppercase that, and finally append % to the string for pattern matching. There isn't anything too complicated in doing this, but 4x backend * 5+ lookups is going to take some time to write with tests.

comment:10 Changed 19 months ago by ronnas@…

  • Summary changed from startswith and contains doesn't work with F expression to startswith endswith and contains doesn't work with F expression

comment:11 Changed 10 months ago by tchaumeny

  • Has patch set
  • Owner changed from nobody to tchaumeny
  • Status changed from new to assigned
  • Version set to master

The current implementation (master) looks definitely broken:

In https://github.com/django/django/pull/3284, PatternLookup is inherited by all "LIKE" lookups and the appropriate pattern_ops are provided for Postgresql, MySQL and SQLite — I don't have an Oracle database to test and there seems to be too much specific code...

Last edited 10 months ago by tchaumeny (previous) (diff)

comment:12 Changed 8 months ago by akaariai

  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me.

comment:13 Changed 8 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 6b5d82749c57a1aae8c9e81d2b85b2cadb9ef933:

Fixed #16731 -- Made pattern lookups work properly with F() expressions

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