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

[Youssef Tarek Ali]

I'm looking into this and plan to provide a patch.

[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

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

[Youssef Tarek Ali]

Replying to 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).

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

[Youssef Tarek Ali]

Replying to 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 ​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 ​the docs). This seems to handle the deprecation nicely without losing index performance on the LHS.