Support mysql query objects as strings in addition to bytes, for PyMySQL support.

[Nathan Klug]

Per ​https://github.com/PyMySQL/PyMySQL/issues/790, Django 2.2 changed the expected type of mysql queries; it's be great if we could support both mysqlclient and PyMySQL here. More details from that issue:

Django 2.1.x used to cast every query into "str" like below
​https://github.com/django/django/blob/stable/2.1.x/django/db/backends/mysql/operations.py#L134

Django 2.2.x they changed the code by using query.decode instead of force_text method.
​https://github.com/django/django/blob/stable/2.2.x/django/db/backends/mysql/operations.py#L140

Could you also help open a ticket on Django's issue tracking system ?
It's relatively easy to fix from Django side.

e.g.

from django.utils.encoding import force_text

def last_executed_query(self, cursor, sql, params):
    # With MySQLdb, cursor objects have an (undocumented) "_executed"
    # attribute where the exact query sent to the database is saved.
    # See MySQLdb/cursors.py in the source distribution.
    query = getattr(cursor, '_executed', None)
    if query is not None:
        if type(query) == bytes:
            query = query.decode(errors='replace')  # mysqlclient
        elif type(query) == str:
            query = query.encode(errors='replace')   # PyMySQL
        else:
            query = force_text(query, errors='replace')  # fallback compatibility ?
    return query

[Mariusz Felisiak]

Thanks for this report, however Django doesn't support PyMySQL (see #12500 and #22391).

[Claude Paroz]

Even if Django does not officially support PyMySQL, I find a bit unfortunate that when a simple fix is available (reverting to force_text usage), we don't even consider it. I think that if at some point PyMySQL support is reachable with minimal efforts, it might be a good thing.

[Mariusz Felisiak]

IMO it is just hard to tell in how many places we have broken PyMySQL support because we don't have a CI build. Moreover even if we'll fix this issue we cannot guarantee that we will not brake anything in the future :| I'm torn.

[Claude Paroz]

Sure I understand, but the fact that this bug was reported may be a sign that we are close to support PyMySQL. It may be time to reevaluate this support.

[Simon Charette]

I agree with Claude here, even if we don't have explicit support confirmed by CI we should try to stay as compatible as possible with PyMySQL.

It already does as a pretty good job at exposing a compatible interface through pymysql.install_as_MySQLdb() and in this particular we're using an undocumented API and expecting it to behave in a certain weird way.

FWIW we ​document we support Oracle's connector but we don't run CI against it and last time I checked support was completely broken and we had to switch to PyMySQL for a project where mysqlclient could not be used.

[Mariusz Felisiak]

ok, agreed. I will partly revert efd8a82e268a82b3ad0be77bd5b4548c30bcb4d7 tomorrow. I can also check if any other test is broken on PyMySQL.

[Mariusz Felisiak]

​PR

[GitHub <noreply@…>]

In a41b092:

Fixed #30380 -- Handled bytes in MySQL backend for PyMySQL support.

This commit partly reverts efd8a82e268a82b3ad0be77bd5b4548c30bcb4d7.

[Tobias Krönke]

Is the partial revert correct? The comment now reads

cursor objects have an (undocumented) "_executed"

but the actual code gets

'_last_executed'

[Mariusz Felisiak]

Thanks Tobias, good catch! I added ​PR.

[GitHub <noreply@…>]

In 994a00e:

Refs #30380 -- Used cursor._executed in DatabaseOperations.last_executed_query() on MySQL.

Regression in a41b09266dcdd01036d59d76fe926fe0386aaade.

Thanks Tobias Krönke for the report.

[Guanzhong Chen]

Is there a plan to backport this change into the 2.2.x branch? It would be nice if this fix could be released in Django 2.2.1.

[eavictor]

Should we open another ticket with Django version 2.2.x and reference to this ticket number for backport ?

[Mariusz Felisiak]

This patch doesn't qualify for a backport.

[Liam]

Hi, bit confused by the release process, when will this fix be available? I can see it wasn't included in 2.2.5, is that correct?

Thanks

[Claude Paroz]

Correct, it will be in Django 3.0, see the Roadmap.