[Django] #17761: save_base() does not properly detect when MTI parent key is unset

Django

unread,

Feb 24, 2012, 10:02:31 AM2/24/12

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset
----------------------------------------------+--------------------
Reporter: agriffis | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.3
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Consider the following MTI scenario, where the primary key of the parent
is a !CharField:

{{{
#!python

from django.db import models

class Foo(models.Model):
id = models.CharField(max_length=6, primary_key=True)

class Bar(Foo):
data = models.TextField()
}}}

Attempting to instantiate and save the child model fails:

{{{
>>> from bar.models import Bar
>>> b=Bar()
>>> b.pk='abcdef'
>>> b.save()
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/base.py", line 460, in save
self.save_base(using=using, force_insert=force_insert,
force_update=force_update)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/base.py", line 553, in save_base
result = manager._insert(values, return_id=update_pk, using=using)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/manager.py", line 195, in _insert
return insert_query(self.model, values, **kwargs)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/query.py", line 1436, in insert_query
return query.get_compiler(using=using).execute_sql(return_id)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/sql/compiler.py", line 791, in execute_sql
cursor = super(SQLInsertCompiler, self).execute_sql(None)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/models/sql/compiler.py", line 735, in execute_sql
cursor.execute(sql, params)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/backends/util.py", line 34, in execute
return self.cursor.execute(sql, params)
File "/home/aron/.virtualenvs/pp/lib/python2.7/site-
packages/django/db/backends/sqlite3/base.py", line 234, in execute
return Database.Cursor.execute(self, query, params)
IntegrityError: bar_bar.foo_ptr_id may not be NULL
}}}

The reason this happens is the following code around line 500 in
[source:django/trunk/django/db/models/base.py db/models/base.py]:

{{{
#!python
class Model(object):
...

def save_base(...):
...

for parent, field in meta.parents.items():
# At this point, parent's primary key field may be unknown
# (for example, from administration form which doesn't fill
# this field). If so, fill it.
if field and getattr(self, parent._meta.pk.attname) is None
and getattr(self, field.attname) is not None:
setattr(self, parent._meta.pk.attname, getattr(self,
field.attname))

self.save_base(cls=parent, origin=org, using=using)
}}}

This fails because the default value for a !CharField is the emtpy string
rather than None. This code works for an !IntegerField primary key (which
defaults to None until set). It also works for a !CharField(null=True)
but
it's not clear to me that's the right answer, because it means null is
valid for the DB column, which isn't right.

I ''think'' the right answer is to change the test from "is None" to "in
[None, !'']" as shown in the attached patch.

You might ask, "What about the test on the child model's pk? Does that
need
to change too?" The answer is no, because the child model's pk is a
!OneToOneField which defaults to None, so only the first None-test on the
line needs to change.

--
Ticket URL: <https://code.djangoproject.com/ticket/17761>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Feb 24, 2012, 10:03:32 AM2/24/12

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------

Reporter: agriffis | Owner: nobody
Type: Bug | Status: new

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by agriffis):

* cc: aron@… (added)
* needs_docs: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_better_patch: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/17761#comment:1>

Django

unread,

Feb 25, 2012, 12:31:21 PM2/25/12

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------

Reporter: agriffis | Owner: nobody
Type: Bug | Status: new

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by ramiro):

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/17761#comment:2>

Django

unread,

Feb 25, 2012, 12:32:51 PM2/25/12

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------

Reporter: agriffis | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 1.3
(models, ORM) | Resolution:

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/17761#comment:3>

Django

unread,

Feb 27, 2012, 11:14:06 AM2/27/12

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------

Reporter: agriffis | Owner: nobody
Type: Bug | Status: new

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by agriffis):

* needs_tests: 1 => 0

Comment:

I've attached a regression test patch, so I'm unchecking "Needs tests"
(though the meaning of that field is unclear, does it mean "issue needs a
test in django" or "ticket needs a test attached?" I'm interpreting it as
the latter)

--
Ticket URL: <https://code.djangoproject.com/ticket/17761#comment:4>

Django

unread,

Jun 13, 2014, 1:52:46 PM6/13/14

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------

Reporter: agriffis | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 1.3
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0

Has patch: 1 | Patch needs improvement: 1

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by timo):

* needs_better_patch: 0 => 1

Comment:

I haven't verified if this is still an issue, but the patch no longer
applies cleanly.

--
Ticket URL: <https://code.djangoproject.com/ticket/17761#comment:5>

Django

unread,

Oct 8, 2015, 8:35:25 PM10/8/15

to django-...@googlegroups.com

#17761: save_base() does not properly detect when MTI parent key is unset

-------------------------------------+-------------------------------------
Reporter: agriffis | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.3
(models, ORM) |

Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by timgraham):