[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>