save(force_update=True) crashes with model inheritance

[Lorenzo Gil Sanchez]

When there is a subclass with no fields and you try to save it with obj.save(force_update=True) it will crash with this exception:

AttributeError: 'NoneType' object has no attribute 'rowcount'

The problem is that an or condition should be an and instead. I'm talking about the check in Model.save_base that test if there are non primary fields to save. This is better explained by the patch itself.

[Lorenzo Gil Sanchez]

Patch that fix this error

[Lorenzo Gil Sanchez]

Simple models that show this problem

[Karen Tracey]

I've attached a diff with a test added to model_inheritance_regress. However, the proposed fix causes other tests in that testcase to fail:

Doctest: regressiontests.model_inheritance_regress.models.__test__.API_TESTS ... FAIL

======================================================================
FAIL: Doctest: regressiontests.model_inheritance_regress.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kmt/django/trunk/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.model_inheritance_regress.models.__test__.API_TESTS
  File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    Place.objects.all()
Expected:
    [<Place: Derelict lot the place>, <Place: Guido's All New House of Pasta the place>]
Got:
    [<Place: Guido's House of Pasta the place>, <Place: Main St the place>]
----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    [sorted(d.items()) for d in dicts] == [[('name', u"Guido's All New House of Pasta"), ('serves_hot_dogs', False)]]
Expected:
    True
Got:
    False
----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    [sorted(d.items()) for d in dicts] == [[('name', u"Guido's All New House of Pasta"), ('serves_gnocchi', False), ('serves_hot_dogs', False)]]
Expected:
    True
Got:
    False
----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    [sorted(d.items()) for d in dicts]
Expected:
    [[('capacity', 50), ('name', u'Derelict lot')]]
Got:
    [[('capacity', 100), ('name', u'Main St')]]
----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    [sorted(d.items()) for d in dicts] == [[('name', u"Guido's All New House of Pasta"), ('serves_gnocchi', False), ('serves_hot_dogs', False)]]
Expected:
    True
Got:
    False
----------------------------------------------------------------------
File "/home/kmt/django/trunk/tests/regressiontests/model_inheritance_regress/models.py", line ?, in regressiontests.model_inheritance_regress.models.__test__.API_TESTS
Failed example:
    Place.objects.get(pk=ident)
Expected:
    <Place: Guido's All New House of Pasta the place>
Got:
    <Place: Guido's House of Pasta the place>


----------------------------------------------------------------------
Ran 1 test in 0.566s

FAILED (failures=1)

Thus it seems the proposed fix is not quite right and a correct fix requires some more investigation.

[Russell Keith-Magee]

Not critical for 1.2

[Klaas van Schelven]

Hi,

I figured I'd take a look at this problem as it was on the list for 1.3.

The offending code is indeed in django/db/models/base.py

522                     if force_update or non_pks:
523                         values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
524                         rows = manager.using(using).filter(pk=pk_val)._update(values)
525                         if force_update and values and not rows:
526                             raise DatabaseError("Forced update did not affect any rows.")

For a subclass with no fields this loop is executed twice, once for the base class and once for its child. The second time values is an empty list, therefore rows is 0 and the exception is raised on line 526. I propose simply adding a check for values, and only raising an exception if values is non-empty. Patch will be attached.

Klaas

[Klaas van Schelven]

Patch w/ fix & tests

[Klaas van Schelven]

Ok, I added the patch and tests that prove the problem exists. I'm not sure why my patch shows so badly in Trac, would gladly resubmit if someone has pointers about that.

[anonymous]

I'd say the #HG comments at the beginning of the patch aren't recognized by trac. Maybe remove them manually ?

[Klaas van Schelven]

Patch w/ tests

[Klaas van Schelven]

meh... still no coloring in the patch... hope this doesn't lead to trouble applying it.

[Karen Tracey]

Replying to vanschelven:

meh... still no coloring in the patch... hope this doesn't lead to trouble applying it.

Try giving the file an extension of .diff

[Jacob]

Milestone 1.3 deleted

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Anssi Kääriäinen]

This is a duplicate of already fixed #13864.