Deprecated usage of BINARY expr in MySQL lookups

[Simon Charette]

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

django/db/backends/mysql/base.py

@@ -164 +164 @@
     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 +186 @@
     # 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('%%', {})",
     }
 

django/db/backends/mysql/operations.py

@@ -363 +363 @@
         # 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"

[Jason Hall]

Would CAST(... AS BINARY) be enough here? This only casts the pattern and not the column? 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?

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

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

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

[Natalia Bidart]

Accepting based on the ​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)

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