Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#9553 closed (invalid)

Save of ForeignKeys's model doesn't cause update of _id field

Reported by: Oldřich Jedlička Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

For two models

class Inner(models.Model):
    field = models.CharField(max_length=10)

class Outer(models.Model):
    inner = models.ForeignKey(Inner)
    other_field = models.CharField(max_length=15)

It is not possible to write

inner = Inner(field="test")
outer = Outer(inner=inner, other_field="other test")
inner.save()
outer.save()

It ends-up with

IntegrityError: null value in column "inner_id" violates not-null constraint

Attachments (1)

django-related_model_fix.patch (1.9 KB) - added by Oldřich Jedlička 8 years ago.
Proof of concept plus test

Download all attachments as: .zip

Change History (8)

comment:1 Changed 8 years ago by Karen Tracey

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: invalid
Status: newclosed

Right, inner does not have a primary key value until you call save() on it. So you cannot correctly specify it as the target of outer's key before then. Your code should be:

inner = Inner(field="test")
inner.save()
outer = Outer(inner=inner, other_field="other test")
outer.save()

I'm pretty sure this is spelled out somewhere in the docs though I can't find it at the moment. The fact that save() is what creates the object's pk is noted here:

http://docs.djangoproject.com/en/dev/ref/models/instances/?from=olddocs#auto-incrementing-primary-keys

comment:2 Changed 8 years ago by Oldřich Jedlička

Resolution: invalid
Status: closedreopened

Actually the problem is not that the inner.save() itself doesn't update the outer's inner_id field, the problem is that the inner_id field is not "intelligently" taken from inner's pk during outer.save(). Outer has all the information to be able to save itself.

To illustrate the problem in detail here is a working code snippet:

inner.save()
outer.inner = outer.inner
outer.save()

My opinion is that this is not the "right thing". If you think this is really wanted behaviour, feel free to close the ticket.

This is from my real-life project (not with Inner and Outer classes, of course). I can live with the workaround, but this doesn't follow the DRY principle.

comment:3 Changed 8 years ago by Oldřich Jedlička

The root cause of the problem is that there are two fields for the same thing:

  • inner_id (Inner's primary key as part of model)
  • inner (Inner instance)

comment:4 Changed 8 years ago by Oldřich Jedlička

Has patch: set

With the attached patch the saving works like a charm. I got inspired by the parent's pk update machinery in Model.save_base and used it to update all RelatedField's pk (that is maybe wrong).

comment:5 Changed 8 years ago by Alex Gaynor

I'm not entirely sure I follow the issue, but either way, can you please include a test in your patch that fails before your patch is applied, and passes afterwords.

Changed 8 years ago by Oldřich Jedlička

Proof of concept plus test

comment:6 Changed 8 years ago by James Bennett

Resolution: invalid
Status: reopenedclosed

This is, I believe, intended behavior: assigning an object to a relation before it has a value for whatever column is used in the relation isn't supposed to work. If you wish to argue for that as a new feature in Django, please take it to the django-developers mailing list rather than endlessly reopening this ticket.

comment:7 Changed 8 years ago by Oldřich Jedlička

All my arguments are here (the model has all the information for save), so let's leave it closed. I do not want to argue for such a stupid thing :-)

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