sqlmigrate fails with string defaults on mysql

[Melvyn Sopacua]

When we have a migration that is adding strings defaults to a field, sqlmigrate will fail:

  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/migrations/migration.py", line 124, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/migrations/operations/fields.py", line 84, in database_forwards
    field,
  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/backends/mysql/schema.py", line 42, in add_field
    super().add_field(model, field)
  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/backends/base/schema.py", line 435, in add_field
    self.execute(sql, params)
  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/backends/base/schema.py", line 128, in execute
    self.collected_sql.append((sql % tuple(map(self.quote_value, params))) + ending)
  File "/home/melvyn/.local/py-env/button3d-7LYQbQGk/lib/python3.6/site-packages/django/db/backends/mysql/schema.py", line 31, in quote_value
    quoted = quoted.decode()
AttributeError: 'str' object has no attribute 'decode'

This is because of the quote_value method calling decode() on a string object (it is a method of a byte object). This doesn't happen when the migration is being applied, presumably, because then we're creating byte objects and the code doesn't trigger, but this is a guess.

[Claude Paroz]

Regression in [3c4ff2176323dd20507e35658599da220fbe1741].

[Claude Paroz]

Melvyn, could you tell us what's the type of your default value? Please also tell us your MySQL and mysqlclient version.

[Claude Paroz]

I was unable to reproduce until now, we need more information, please.

[Melvyn Sopacua]

If you can't reproduce it, you have dead code, cause str.decode() simply isn't a thing.

However, it may be caused by an extension. I will have time on Friday to investigate.

[Claude Paroz]

The code counts on the fact that connection.escape() will return a bytestring for a str value. Note the isinstance test is done on value but the decode() is called on quoted.

[Melvyn Sopacua]

Thanks!
Note to self: test 2 cases:

• migrations without any extensions and/or foreign migration operations
• compare mysql driver versions: pymysql 0.9.3

Also check connection.escape().

[Claude Paroz]

Replying to Melvyn Sopacua:

• compare mysql driver versions: pymysql 0.9.3

Ah, ah... that is probably the explanation. It might be that pymysql doesn't return a bytestring from quote() which would explain the issue (to be confirmed). Django doesn't officially support pymysql (read also #30380). It's another issue where we might call force_text instead of decode(). However this section is a bit more performance-sensitive.

[Melvyn Sopacua]

I think the proper fix here is to confirm both str on value and 'decode' attribute on quoted.

I've tried to add a test for it, but the code never gets called anymore in the scope of migrations, because Django no longer transmits default values to the backend. So the issue is with ​our extension that ensures we can do blue/green deployments. Looking at ​this code confirms your suspicions that pymysql returns strings.

Still - when I read the code the first few times until you highlighted the fact here, I thought the if clause and the modification are using the same value, cause it's such a common pattern you don't read the variable name anymore.
And secondly, it assumes that quoted has the decode() method based on a not related test, so I still suggest to fix it like this:

if isinstance(value, str) and callable(getattr(quoted, 'decode', None)):
    quoted = quoted.decode()

This doesn't require the performance hit of force_text and it even supports exotic quoted values that implement a decode() method.

[Mariusz Felisiak]

This should be fixed as a part of a41b09266dcdd01036d59d76fe926fe0386aaade.