Directly creating model instances on ForeignKey field fails

[direx]

Example models.py:

class Author(models.Model):
    name = models.CharField(max_length=100)

class Book(models.Model):
    title  = models.CharField(max_length=100)
    author = models.ForeignKey(Author, null=True)

Code triggering the bug:

b=Book()
b.title="Working title"
b.author=Author()
b.author.name="Foo Bar"
b.author.save()
b.save()

What should happen:

• The Author model is being saved first
• The Book model is being saved second, with the created Author instance set as a ForeignKey instance.

What really happens:

• Well, the Author and Book models are saved in the expected order, without any errors, but in the database the author property is NULL. This means that information is silently lost!

Some more words:
If the author attribute had null=False set, an IntegrityError would be raised (author_id may not be NULL), which is better than failing silently, but still bad. The issue with this behavior is that from a normal user's/developer's point of view (without deep knowledge of Django's guts) there is no difference between the following code and the example code above (except that the following code works as expected):

b=Book()
b.title="Working title"
a=Author()
a.name="Foo Bar"
a.save()
b.author=a
b.save()

Of course this can be worked around by end-users by setting b.author_id to b.author.id, but that does not seem logical at all and violates the DRY principle. The real issue probably is the magic triggered by b.author=a. Or Django's ForeignKey lookup is a bit too lazy here...

[Anssi Kääriäinen]

To me it seems that when saving the b in above example we could easily and cheaply check that if b.author_id is null, then check if b.author has id, and if so use that.

There is one potential backwards compatibility issue:

b = B.objects.get(somecond)
assert(b.author.id == b.author_id) # it has an author, the fields have same value 
b.author_id = None
b.save() # The b.author_id is None, so refetch the author_id from b.author.id

This is different from what happens currently in Django (where the author_id would be set to NULL), and could lead to working code suddenly working subtly differently, possibly including data corruption. Seems like the worst case of backwards incompatibility to me.

So, this must be worked around. Maybe save of b.author could somehow set b.author_id when b.author.id is set (but I don't believe the author has currently reference to b). Or maybe there is some other way to detect that b.author has gotten different id, but b.author_id hasn't changed. Still, the solution should be fast enough to not have any dramatic memory or performance requirements (mainly, Model.__init__ could be a problem).

I am going to mark this accepted, but the patch needs to deal with the above mentioned backwards incompatibility somehow. This might be surprisingly hard to solve correctly.

(The problem of b.author.id != b.author_id is more generic - a solution where it is possible to assert that the user sees always the same value for these fields while performance isn't compromised is welcome...)

[Daniele Procida]

I am not convinced that this should be considered a bug. The behaviour may be a little counter-intuitive at first sight, but I don't see that it's actually wrong.

[drtyrsa]

I think it's better to document the existing behavior properly. Points to mention:

1. You should save the instance before assigning it as an attribute.
2. You can event set foo_id attribute directly, but it is less safe, as some checks will be skipped. Although it sometimes can be more efficient, as there is no need to fetch foo instance from database.

[Tim Graham]

Duplicate of #10811