[Django] #36571: Deprecated usage of BINARY expr in MySQL lookups

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Normal
Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
When enabling warnings against MySQL usage of `__contains`,
`__startswith`, `__endswith` on MySQL result in the following warning

> Warning 1287 "'BINARY expr' is deprecated and will be removed in a
future release

It seems like a potential solution is simply to use `CAST` instead

{{{#!diff
diff --git a/django/db/backends/mysql/base.py
b/django/db/backends/mysql/base.py
index e83dc106f7..236e6c599e 100644
--- a/django/db/backends/mysql/base.py
+++ b/django/db/backends/mysql/base.py
@@ -164,14 +164,14 @@ def data_types(self):
operators = {
"exact": "= %s",
"iexact": "LIKE %s",
- "contains": "LIKE BINARY %s",
+ "contains": "LIKE CAST(%s AS BINARY)",
"icontains": "LIKE %s",
"gt": "> %s",
"gte": ">= %s",
"lt": "< %s",
"lte": "<= %s",
- "startswith": "LIKE BINARY %s",
- "endswith": "LIKE BINARY %s",
+ "startswith": "LIKE CAST(%s AS BINARY)",
+ "endswith": "LIKE CAST(%s AS BINARY)",
"istartswith": "LIKE %s",
"iendswith": "LIKE %s",
}
@@ -186,11 +186,11 @@ def data_types(self):
# wildcard for the LIKE operator.
pattern_esc = r"REPLACE(REPLACE(REPLACE({}, '\\', '\\\\'), '%%',
'\%%'), '_', '\_')"
pattern_ops = {
- "contains": "LIKE BINARY CONCAT('%%', {}, '%%')",
+ "contains": "LIKE CAST(CONCAT('%%', {}, '%%') AS BINARY)",
"icontains": "LIKE CONCAT('%%', {}, '%%')",
- "startswith": "LIKE BINARY CONCAT({}, '%%')",
+ "startswith": "LIKE CAST(CONCAT({}, '%%') AS BINARY)",
"istartswith": "LIKE CONCAT({}, '%%')",
- "endswith": "LIKE BINARY CONCAT('%%', {})",
+ "endswith": "LIKE CAST(CONCAT('%%', {}) AS BINARY)",
"iendswith": "LIKE CONCAT('%%', {})",
}

diff --git a/django/db/backends/mysql/operations.py
b/django/db/backends/mysql/operations.py
index 7dfcd57958..bdde5bbf47 100644
--- a/django/db/backends/mysql/operations.py
+++ b/django/db/backends/mysql/operations.py
@@ -363,7 +363,7 @@ def regex_lookup(self, lookup_type):
# REGEXP_LIKE doesn't exist in MariaDB.
if self.connection.mysql_is_mariadb:
if lookup_type == "regex":
- return "%s REGEXP BINARY %s"
+ return "%s REGEXP CAST(%s AS BINARY)"
return "%s REGEXP %s"

match_option = "c" if lookup_type == "regex" else "i"
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36571>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:

Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jason Hall):

Would `CAST(... AS BINARY)` be enough here? This only casts the pattern
and no the column? That might lead to subtle differences depending on the
column's collation or affect index usage. `COLLATE ..._bin` be closer to
the old `LIKE BINARY`, since it applies a binary collation to the column
and the pattern?
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:1>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

> Would `CAST(... AS BINARY)` be enough here? This only casts the pattern

and not the column?

I think you're right, we'd need to cast both the left and right hand side
of the operator.

> That might lead to subtle differences depending on the column's

collation or affect index usage. Would `COLLATE ..._bin` be closer to the

old `LIKE BINARY`, since it applies a binary collation to the column and
the pattern?

The problem with that is that it requires knowing the right-hand-side's
collation at query execution time and append the `..._bin` suffix to it.

I guess we could use `rhs.output_field.db_collation` and default to
`utf8mb_bin` if missing but that could also subtly break as there could be
databases out there with collations that are not matching the Django model
representation of it and were working fine previously when using `LIKE
BINARY`.

This whole thing makes me wonder why they deprecated this option in the
first place as it seems quite handy.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:2>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jason Hall):

* owner: (none) => Jason Hall
* status: new => assigned

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

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jason Hall):

I've opened a draft PR for this change. At the moment, most of the test
suite passes using MySQL but I'm still seeing two failing tests:
`expressions.tests.ExpressionsTests.test_patterns_escape`
`expressions.tests.ExpressionsTests.test_insensitive_patterns_escape`

These both deal with `%` characters in expressions. Literal values
`("Article%")` behave correctly with escaping, but expressions like
`F("lastname")` containing `%` are failing.

I've tried a few different approaches to unify the escaping rules so
everything passes, but each time i fix one side it tends to blow up
elsewhere in the suite. Instead of continuing to hack away at this, I
thought i'd take a break and put up a PR in it's current state to get some
feedback.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:4>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jason Hall):

* has_patch: 0 => 1
* needs_better_patch: 0 => 1

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

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jason Hall):

{{{
escaped_rhs = (
"REPLACE("
" REPLACE("
" REPLACE({rhs}, CHAR(92), CONCAT(CHAR(92),
CHAR(92))),"
" CHAR(37), CONCAT(CHAR(92), CHAR(37))"
" ),"
" CHAR(95), CONCAT(CHAR(92), CHAR(95))"
")"
).format(rhs=rhs)
}}}

I was seing failures in
`expressions.tests.ExpressionsTests.test_patterns_escape` and
`test_insensitive_patterns_escape` because `%`, `_`, and `\` were not
being escaped correctly when the RHS of a pattern lookup was an expression
(e.g. `F("lastname")` ).

Using `'\\\\'` and `'%%%%'` produced incorrect escaping once Django
interpolated the SQL string. I switched to the above code using `CHAR()`
and all the relevant tests seem to be passing when using MySQL.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:6>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev

(models, ORM) |
Severity: Normal | Resolution:

Keywords: mysql binary like | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted
* version: 5.2 => dev

Comment:

Accepting based on the
[https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-27.html MySQL
8.0.27 release notes]:

> Important Change: The BINARY operator is now deprecated, and subject to
removal in a future release of MySQL. Use of BINARY now causes a warning.
Use CAST(... AS BINARY) instead. (WL #13619)
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:7>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Jason
Type: | Hall
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jason Hall):

i accidently modified the wrong file.. I had the mysql stuff working but
the modifications I made were in django/db/models/lookups.py. So those
changes blew up all the sqlite tests. Removed those changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:8>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jason Hall):

* owner: Jason Hall => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:9>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner:

Type: | JaeHyuckSa
Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* owner: (none) => JaeHyuckSa

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:10>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by JaeHyuckSa):

* owner: JaeHyuckSa => (none)

* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:11>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Youssef Tarek Ali):

* owner: (none) => Youssef Tarek Ali

* status: new => assigned

Comment:

I'm looking into this and plan to provide a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:12>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Youssef Tarek Ali):

I've opened a PR to handle this deprecation by switching to `CAST(... AS
BINARY)` in the MySQL backend.

I made sure to only cast the RHS parameters to keep index performance
high. I've also run the full `expressions` and `lookup` test suites (337
tests) on both MySQL 9.0 and MariaDB latest to make sure there are no
regressions, especially with the complex escaping cases that were
mentioned in earlier discussions.

PR link: https://github.com/django/django/pull/20865
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:13>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

> I made sure to only cast the RHS parameters to keep index performance
high.

Wasn't the conclusion of the previous discussion that we cannot do just
the RHS?
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:14>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Youssef Tarek Ali):

Replying to [comment:14 Adam Johnson]:

> > I made sure to only cast the RHS parameters to keep index performance
high.
>
> Wasn't the conclusion of the previous discussion that we cannot do just
the RHS?

I've verified in a Docker environment (MySQL 9.0 with
`utf8mb4_general_ci`) that casting only the RHS (e.g. `LIKE CAST(%s AS
BINARY)`) correctly forces a case-sensitive comparison. This aligns with
the MySQL documentation https://dev.mysql.com/doc/refman/8.0/en/string-
comparison-functions.html#operator_like, which states: "A comparison
between a character string and a binary string is treated as a comparison
of binary strings."

The primary advantage of this approach is that it preserves index usage by
leaving the LHS column untransformed. I have cross-verified this fix by
running the full `expressions` and `lookup` test suites (337 tests) on
both MySQL 9.0 and MariaDB latest backends, and all tests passed
(including `test_patterns_escape`).
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:15>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

I have to note that while I agree (in comment:2) with comment:1 in theory
I never cross verified it myself so it's entirely possible that this
simple approach just work.

All I can say is that we've been using an approach similar to the `CAST`
approach at work and it has been working fine for us, we don't use exotic
collations though (we stick to `utf8_general_ci`).

Something is definitely weird with this deprecation as MySQL's `LIKE`
documentation [https://dev.mysql.com/doc/refman/9.6/en/string-comparison-
functions.html#operator_like still mentions] the usage of `BINARY` to
force case-sensitive comparison. From my understanding we have gone the
extra length of doing so on MySQL because the historical default
collations are all case-insensitive (e.g. `utf8_general_ci`) and breaking
with this legacy by forcing users to move a case-sensitive collation on a
new version of Django would be quite disruptive even with the relatively
recent introduction of `Field(db_collation)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:16>

[Django]

#36571: Deprecated usage of BINARY expr in MySQL lookups
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Youssef
Type: | Tarek Ali
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: mysql binary like | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Youssef Tarek Ali):

Replying to [comment:16 Simon Charette]:

> I have to note that while I agree (in comment:2) with comment:1 in
theory I never cross verified it myself so it's entirely possible that
this simple approach just work.
>
> All I can say is that we've been using an approach similar to the `CAST`
approach at work and it has been working fine for us, we don't use exotic
collations though (we stick to `utf8_general_ci`).
>
> Something is definitely weird with this deprecation as MySQL's `LIKE`
documentation [https://dev.mysql.com/doc/refman/9.6/en/string-comparison-
functions.html#operator_like still mentions] the usage of `BINARY` to
force case-sensitive comparison. From my understanding we have gone the
extra length of doing so on MySQL because the historical default
collations are all case-insensitive (e.g. `utf8_general_ci`) and breaking
with this legacy by forcing users to move a case-sensitive collation on a
new version of Django would be quite disruptive even with the relatively
recent introduction of `Field(db_collation)`.

I verified this with a MySQL 9.0 Docker setup on `utf8mb4_general_ci`. The
RHS-only `CAST` is case-sensitive because MySQL treats comparisons with a
binary string as binary (per [https://dev.mysql.com/doc/refman/8.0/en
/string-comparison-functions.html#operator_like the docs]). This seems to
handle the deprecation nicely without losing index performance on the LHS.
--
Ticket URL: <https://code.djangoproject.com/ticket/36571#comment:17>