Opened 5 years ago

Closed 22 months ago

#16731 closed Bug (fixed)

startswith endswith and contains doesn't work with F expression

Reported by: ronnas@… Owned by: Thomas Chaumeny
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: startswith, F(), wildcards
Cc: Koen Vossen 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 Aymeric Augustin)

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 Marc Tamlyn 3 years ago.
16731-failing-test.patch

Download all attachments as: .zip

Change History (14)

comment:1 Changed 5 years ago by Aymeric Augustin

Description: modified (diff)
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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 5 years ago by anonymous

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

comment:3 Changed 3 years ago by Koen Vossen

Cc: Koen Vossen 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 3 years ago by Marc Tamlyn

Attachment: 1117.patch.txt added

16731-failing-test.patch

comment:4 Changed 3 years ago by Marc Tamlyn

Version: 1.3

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 3 years ago by uberj@…

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

comment:6 Changed 3 years ago by Matthias Meulien

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 3 years ago by Matthias Meulien (previous) (diff)

comment:7 Changed 3 years ago by Tim Graham

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 3 years ago by Matthias Meulien

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 3 years ago by Matthias Meulien (previous) (diff)

comment:9 Changed 3 years ago by Anssi Kääriäinen

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 3 years ago by ronnas@…

Summary: startswith and contains doesn't work with F expressionstartswith endswith and contains doesn't work with F expression

comment:11 Changed 2 years ago by Thomas Chaumeny

Has patch: set
Owner: changed from nobody to Thomas Chaumeny
Status: newassigned
Version: 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 2 years ago by Thomas Chaumeny (previous) (diff)

comment:12 Changed 22 months ago by Anssi Kääriäinen

Triage Stage: AcceptedReady for checkin

Looks good to me.

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

Resolution: fixed
Status: assignedclosed

In 6b5d82749c57a1aae8c9e81d2b85b2cadb9ef933:

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

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