Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate keyword '_binary' failure when using BinaryField in Django #549

Closed
ace-han opened this issue Feb 10, 2017 · 8 comments · Fixed by #628
Closed

Duplicate keyword '_binary' failure when using BinaryField in Django #549

ace-han opened this issue Feb 10, 2017 · 8 comments · Fixed by #628

Comments

@ace-han
Copy link

ace-han commented Feb 10, 2017

Process to reproduce

# models.py
from django.db import models
from django.utils.translation import ugettext_lazy as _

class DbBasedFile(models.Model):
  filename = models.CharField(_('filename'), max_length=128)
  content = models.BinaryField(_('content'))
  size = models.PositiveIntegerField(_('size'))

  class Meta:
      unique_together = (('filename', ), )
  
  def __str__(self):
      return '{}, filename: {}'.format(self.__class__.__name__, self.filename)

if __name__ == '__main__':
  DbBasedFile(filename='1.txt', content=b'abc', size=3).save()

will get error

ProgrammingError at /api/misc/dbfiles/file/
(1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '_binary'abc', 3)'

The pre-generated sql is like below

str: INSERT INTO `misc_dbbasedfile` (`filename`, `content`, `size`) VALUES (%s, _binary %s, %s)

The final sql is like below

bytes: b"INSERT INTO `misc_dbbasedfile` (`filename`, `content`, `size`) VALUES ('1.txt', _binary _binary'abc', 3)"

As we can see, there are two _binary in the final sql which will fail eventually
After a little debugging, I found these _binary in

django/django/db/backends/mysql/operations.py.DatabaseOperations.binary_placeholder_sql
django/db/models/fields/__init__.py.BinaryField.get_placeholder
pymysql/converters.py.escape_bytes

, which I think is the root cause

I think we can refer to below links to get a solution,
PyMySQL/mysqlclient#106
PyMySQL/mysqlclient#140

Maybe making it optional to add _binary prefix is a nice idea 😄

PyMySQL is great in its pure python implementation to ease the installation on diff OS platform.
I really want this to be fixed, please kindly help, thx.

@lambdalisue
Copy link

For note, a temporary solution is pip install —upgrade 'PyMySQL<0.7.0' (Thanks to @giginet)

@kinganu
Copy link

kinganu commented Apr 27, 2017

hi can i ask how you got pymysql working with django in first place, have a connection doesnt exist error, details here: https://dpaste.de/8Oya . also got error:

django.core.exceptions.ImproperlyConfigured: Error loading MySQLdb module: No module named 'MySQLdb'

@eavictor
Copy link
Contributor

eavictor commented Jun 7, 2017

Anyone test on 0.7.11 ?

The solution for me is use "ImageField" and store images out from database.

@kinganu Here you go.
Place the method in settings.py

import pymysql

pymysql.install_as_MySQLdb()

@troyjfarrell
Copy link
Contributor

troyjfarrell commented Sep 28, 2017

@ace-han As you discovered in your Django issue , the mysqlclient-python fix for this issue PyMySQL/mysqlclient#106 is to hide the additional _binary classifier behind an option flag binary_prefix so as to not break the existing Django implementation.

We can do this in PyMySQL, but it's made more difficult by the fact that the escape_* functions do not have access to the Connection object. We could set a global, but that would be ugly.

@methane
Copy link
Member

methane commented Sep 29, 2017

Yes. Current conv API (with escape_* functions) is ugly, incomplete, broken.
It's hard to maintain. I want to remove it.

But Django uses it....

@eavictor
Copy link
Contributor

I will suggest PyMySQL can setup a time to deprecate and remove this method in near future.
Let Django decides if they wants to follow up.

Ugly fix/implementation is not good +1

@ace-han
Copy link
Author

ace-han commented Dec 26, 2017

@methane How do I integrate with Django with binary_prefix=True or even better control sometimes binary_prefix=True sometimes binary_prefix=False 😈

Anything adds to below in settings.py for Django?

import pymysql

pymysql.install_as_MySQLdb()

@methane
Copy link
Member

methane commented Dec 26, 2017

I don't know Django. I'm not Django user.
But I think deafault setting is good for Django.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants