[Django] #35732: Postgresql Concat using || and Trigram similarity operator precedence bug

15 views
Skip to first unread message

Django

unread,
Sep 5, 2024, 9:03:48 AM9/5/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
--------------------------+--------------------------------------------
Reporter: avilaton | Type: Bug
Status: new | Component: contrib.postgres
Version: 5.0 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------+--------------------------------------------
A change from 5.0.8 to 5.1 raised a test failure in one of our queries
that combines Concat with TrigramSimilarity. @emicuencac tracked this down
to a recently deployed simplification on how Concat is rendered to sql
here #17471 which leads to the pipe operator not being wrapped in
parenthesis which were implicit when using CONCAT(...).
Here is a more explicit example
{{{
Model.objects
.annotate(
concat_result=Concat(F("field"), V("tew")),
similarity=TrigramSimilarity("concat_result", search_term),
)
.filter(concat_result__trigram_similar=search_term)
.values("field"),
[{"field": "Matthew"}],
}}}
which works well with django 5.0.8 but fails in 5.1. It fails because the
mentioned change renders CONCAT using the `||` operator without wrapping
parenthesis and ends up sending something like this to the DB
which would render this before the change
{{{
WHERE CONCAT('something', 'other_word') % 'search_term'
}}}
but now renders
{{{
WHERE 'something' || 'other_word' % 'search_term'
}}}
which breaks the query because the similarity operator is evaluated first.

The error that looks like this
{{{
def execute(
self,
query: Query,
params: Params | None = None,
*,
prepare: bool | None = None,
binary: bool | None = None,
) -> Self:
"""
Execute a query or command to the database.
"""
try:
with self._conn.lock:
self._conn.wait(
self._execute_gen(query, params, prepare=prepare,
binary=binary)
)
except e._NO_TRACEBACK as ex:
> raise ex.with_traceback(None)
E django.db.utils.ProgrammingError: argument of WHERE must be
type boolean, not type text
E LINE 1: ...e" FROM "suggest_vins_makemodelsearchentry" WHERE
COALESCE("...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35732>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Sep 5, 2024, 9:31:29 AM9/5/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
----------------------------------+--------------------------------------
Reporter: avilaton | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------------------------
Comment (by Natalia Bidart):

Hola Gastón, thank you for your report!

Could you please provide specific details of the `Model` definition that
you are using, so we can triage this ticket accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/35732#comment:1>

Django

unread,
Sep 5, 2024, 10:28:35 AM9/5/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
----------------------------------+------------------------------------
Reporter: Gastón Avila | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Simon Charette):

* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* version: 5.0 => 5.1


Old description:
New description:

A change from 5.0.8 to 5.1 raised a test failure in one of our queries
that combines Concat with TrigramSimilarity. @emicuencac tracked this down
to a recently deployed simplification on how Concat is rendered to sql
here #34955 which leads to the pipe operator not being wrapped in
Comment:

Regression in 6364b6ee1071381eb3a23ba6b821fc0d6f0fce75.
--
Ticket URL: <https://code.djangoproject.com/ticket/35732#comment:2>

Django

unread,
Sep 6, 2024, 8:02:11 AM9/6/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
----------------------------------+------------------------------------
Reporter: Gastón Avila | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Natalia Bidart):

* needs_docs: 0 => 1

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

Django

unread,
Sep 6, 2024, 8:26:32 AM9/6/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
----------------------------------+------------------------------------
Reporter: Gastón Avila | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Gastón Avila):

* needs_docs: 1 => 0

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

Django

unread,
Sep 9, 2024, 3:01:48 AM9/9/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
----------------------------------+----------------------------------------
Reporter: Gastón Avila | Owner: Gastón Avila
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+----------------------------------------
Changes (by Sarah Boyce):

* owner: (none) => Gastón Avila
* status: new => assigned

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

Django

unread,
Sep 11, 2024, 8:36:25 AM9/11/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
-------------------------------------+-------------------------------------
Reporter: Gastón Avila | Owner: Gastón
| Avila
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35732#comment:6>

Django

unread,
Sep 11, 2024, 8:37:05 AM9/11/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
-------------------------------------+-------------------------------------
Reporter: Gastón Avila | Owner: Gastón
| Avila
Type: Bug | Status: closed
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

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

Comment:

In [changeset:"c3ca6075cc0ad425bcf905fe14062f38eb9fbcbf" c3ca607]:
{{{#!CommitTicketReference repository=""
revision="c3ca6075cc0ad425bcf905fe14062f38eb9fbcbf"
Fixed #35732 -- Wrapped ConcatPair expression in parentheses to ensure
operator precedence.

When ConcatPair was updated to use || this lost the implicit wrapping from
CONCAT(...).
This broke the WHERE clauses when used in combination with PostgreSQL
trigram similarity.

Regression in 6364b6ee1071381eb3a23ba6b821fc0d6f0fce75.

Co-authored-by: Emiliano Cuenca
<106986074+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35732#comment:7>

Django

unread,
Sep 11, 2024, 8:41:23 AM9/11/24
to django-...@googlegroups.com
#35732: Postgresql Concat using || and Trigram similarity operator precedence bug
-------------------------------------+-------------------------------------
Reporter: Gastón Avila | Owner: Gastón
| Avila
Type: Bug | Status: closed
Component: contrib.postgres | Version: 5.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"590f5e09f001736aa1fe3f7f086f8a5ab5e4bfdb" 590f5e0]:
{{{#!CommitTicketReference repository=""
revision="590f5e09f001736aa1fe3f7f086f8a5ab5e4bfdb"
[5.1.x] Fixed #35732 -- Wrapped ConcatPair expression in parentheses to
ensure operator precedence.

When ConcatPair was updated to use || this lost the implicit wrapping from
CONCAT(...).
This broke the WHERE clauses when used in combination with PostgreSQL
trigram similarity.

Regression in 6364b6ee1071381eb3a23ba6b821fc0d6f0fce75.

Backport of c3ca6075cc0ad425bcf905fe14062f38eb9fbcbf from main.

Co-authored-by: Emiliano Cuenca
<106986074+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35732#comment:8>
Reply all
Reply to author
Forward
0 new messages