Opened 8 years ago

Closed 8 years ago

#27419 closed Bug (needsinfo)

Model that worked before 1.10 causes "Cannot force an update in save() with no primary key." in 1.10.

Reported by: Louis-Dominique Dubeau Owned by: nobody
Component: Database layer (models, ORM) Version: 1.10
Severity: Normal Keywords: save model property
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This problem does not happen prior to Django 1.10. I'm running this on Python 2.7.12 but I don't think the Python version is an issue. The following case is distilled form a full-fledged application that has run fine in production since Django 1.6. Upon upgrading to 1.10, I immediately got the error reported here.

Consider the following models.py file:

from __future__ import unicode_literals

from django.db import models

class Foo(models.Model):

    a = models.CharField(max_length=10)
    b = models.CharField(max_length=10)
    parent = models.ForeignKey(
        "self", related_name="children", null=True, blank=True)

class Bar(models.Model):

    a = models.CharField(max_length=10)
    _b = models.CharField(max_length=10, name="b", db_column="b")
    parent = models.ForeignKey(
        "self", related_name="children", null=True, blank=True)

    @property
    def b(self):
        return self._b

    @b.setter
    def b(self, val):
        print "SETTING B"
        # This would make the problem disappear:
        # self.__dict__["b"] = val
        self._b = val

And the following tests.py file which is a sibling to models.py above:

from django.test import TestCase

from .models import *

class TestFoo(TestCase):

    def test(self):
        parent = Foo(a="parent_a", b="parent_b")
        parent.save()
        foo = Foo(a="1a", b="1b", parent=parent)
        foo.save()

class TestBar(TestCase):

    def test(self):
        parent = Bar(a="parent_a", b="parent_b")
        parent.save()
        bar = Bar(a="1a", b="1b", parent=parent)
        bar.save()

Running ./manage.py test results in:

$ ./manage.py test
Creating test database for alias 'default'...
SETTING B
SETTING B
E.
======================================================================
ERROR: test (myapp.tests.TestBar)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ldd/src/django_issues/deferred_fields_in_1.10/issue/myapp/tests.py", line 19, in test
    bar.save()
  File "/home/ldd/src/django_issues/deferred_fields_in_1.10/issue-venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 796, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/ldd/src/django_issues/deferred_fields_in_1.10/issue-venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 824, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/home/ldd/src/django_issues/deferred_fields_in_1.10/issue-venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 880, in _save_table
    raise ValueError("Cannot force an update in save() with no primary key.")
ValueError: Cannot force an update in save() with no primary key.

----------------------------------------------------------------------
Ran 2 tests in 0.002s

FAILED (errors=1)
Destroying test database for alias 'default'...

If I downgrade to Django 1.9, I get:

$ ./manage.py test
Creating test database for alias 'default'...
SETTING B
SETTING B
..
----------------------------------------------------------------------
Ran 2 tests in 0.001s

OK
Destroying test database for alias 'default'...

Attachments (2)

models.py (726 bytes ) - added by Louis-Dominique Dubeau 8 years ago.
tests.py (428 bytes ) - added by Louis-Dominique Dubeau 8 years ago.

Download all attachments as: .zip

Change History (5)

by Louis-Dominique Dubeau, 8 years ago

Attachment: models.py added

by Louis-Dominique Dubeau, 8 years ago

Attachment: tests.py added

comment:1 by Tim Graham, 8 years ago

It's not obvious that this is a Django bug since you're doing some non-standard stuff with setter. Could you try bisecting to find the commit where the behavior changed? A similar issue was reported on django-users.

comment:2 by Lucas Moeskops, 8 years ago

The bug seems to be introduced here:

commit 7f51876f99851fdc3fef63aecdfbcffa199c26b9
Author: Anssi Kääriäinen <anssi.kaariainen@thl.fi>
Date:   Tue Feb 2 11:33:09 2016 +0200

    Fixed #26207 -- Replaced dynamic classes with non-data descriptors for deferred instance loading.

:040000 040000 bc475a97153a13ed83629d19d53f4aff0cb064f6 0343367b1a537396c570ab15d6c8495b70e4700c M	django
:040000 040000 439540406e70c8b9ef43db729dbec4cb473e7ee0 a4775e8e1bdb64eb549d000a9e6efcd58868b86e M	docs
:040000 040000 cc14ca4b983abe3a6aa6f0f82b329375e7259df7 a9e3feb171711fe29098f1f3d2370f878f936c04 M	tests

comment:3 by Lucas Moeskops, 8 years ago

Resolution: needsinfo
Status: newclosed

It seems to have to do with specifying a different name attribute ('b') on the field than the field name itself ('_b'). If I omit that attribute, the tests pass. I can't find documentation for passing a different name attribute. Maybe this is incorrect?

Note: See TracTickets for help on using tickets.
Back to Top