Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#35732 closed Bug (fixed)

Postgresql Concat using || and Trigram similarity operator precedence bug

Reported by: Gastón Avila Owned by: Gastón Avila
Component: contrib.postgres Version: 5.1
Severity: Release blocker Keywords:
Cc: Gastón Avila 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 Simon Charette)

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 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("...

Change History (8)

comment:1 by Natalia Bidart, 2 months ago

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.

EDIT: I see now, after taking a quick look at your PR, that you used the Model as defined in the test suite, thanks.

Last edited 2 months ago by Natalia Bidart (previous) (diff)

comment:2 by Simon Charette, 2 months ago

Description: modified (diff)
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted
Version: 5.05.1

comment:3 by Natalia Bidart, 2 months ago

Needs documentation: set

comment:4 by Gastón Avila, 2 months ago

Needs documentation: unset

comment:5 by Sarah Boyce, 2 months ago

Owner: set to Gastón Avila
Status: newassigned

comment:6 by Sarah Boyce, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In c3ca607:

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+emicuencac@…>

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

In 590f5e0:

[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+emicuencac@…>

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