Foreign Keying To A Field With An Empty String Value Converts To Null Database Value

[albertyw@…]

If I have these two models:

class A(models.Model):
    b = models.ForeignKey(B)

class B(models.Model):
    asdf = models.CharField(max_length = 10, primary_key=True)

then

empty = B()
empty.asdf = ''
empty.save()
alpha = A()
alpha.b = empty
alpha.save()

will break because django will try to convert the asdf_id field to NULL when creating the sql statement. (I'm running MySQL, but I'm guessing this affects non-mysql databases)

The traceback is

  File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py", line 546, in save
    force_update=force_update, update_fields=update_fields)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py", line 626, in save_base
    rows = manager.using(using).filter(pk=pk_val)._update(values)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 605, in _update
    return query.get_compiler(self.db).execute_sql(None)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/sql/compiler.py", line 1018, in execute_sql
    cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/sql/compiler.py", line 840, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/util.py", line 41, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/mysql/base.py", line 120, in execute
    return self.cursor.execute(query, args)
  File "/usr/lib/python2.7/dist-packages/MySQLdb/cursors.py", line 176, in execute
    if not self._defer_warnings: self._warning_check()
  File "/usr/lib/python2.7/dist-packages/MySQLdb/cursors.py", line 92, in _warning_check
    warn(w[-1], self.Warning, 3)
_mysql_exceptions.Warning: Column 'base_element_id' cannot be null

I've specifically found that the get_db_prep_save function in db/models/fields/related.py:1042 is changing the empty string value over to None, which then gets converted to NULL in sql. I also noticed there's an unused "empty_strings_allowed = False" in that function's class.

[anonymous]

I added a patch at ​https://github.com/django/django/pull/1690

[Baptiste Mispelon]

Hi,

Thanks for the detailed report. I can indeed reproduce this under sqlite too.

I'll add that this affects master too, and doesn't appear to be a regression (the bug is present in older versions of django as well).

[Baptiste Mispelon]

Regarding your patch, I'm not familiar enough with the ORM code to review it in depth but I did give it a quick look.

It does seem to fix the issue you're reporting and it passes the existing test suite (with sqlite).

However, it lacks tests of its own: see our contributor guide for more info [1].

Thanks.

[1] ​https://docs.djangoproject.com/en/1.5/internals/contributing/writing-code/submitting-patches/#patch-style

[anonymous]

I'll add in tests soon then. I'm not entirely sure my code is correct either. I may need to check for TextField in addition to CharField, but I'm under the impression that you can't foreign key to a text field anyways (or that may be database specific).

[albertyw@…]

I added a test for it in the latest commit in the pull request (and made a fix suggested in a pull request comment).

[Baptiste Mispelon]

Hi,

Your updated pull request looks much better now: good job!
I've made a few "stylistic" comments but overall it's pretty good I think.

I'll try to get someone better-versed in the ORM internals to review this, just to make sure I'm not missing anything.

Thanks for your contribution.

[anonymous]

ping on this?

[Tim Graham]

I believe this is a duplicate of #19299. That ticket has a slightly different patch, so maybe you could take a look there and see which approach you think is correct.