Opened 10 years ago

Closed 9 years ago

#22726 closed Bug (duplicate)

Prevent setting nullable relations on unsaved objects

Reported by: Vlastimil Zíma Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Dispute on resolution of #18153 and #19089.

Assume you have models Place and UndergroundBar with nullable one-to-one relation to the place, as in one_to_one_regress tests models.

Since Django 1.6 the one-to-one relation on unsaved objects became asymmetric. You could set one-to-one relation, but not the reverse one-to-one relation, even though setting forward relation sets the cache for reverse relation.

Example:

place = Place()
bar = UndergroundBar()

place.undergroundbar = bar # Raises error
# But
bar.place = place
place.undergroundbar # Returns 'bar'

I attach patch with fix and modification of regression test test_set_reverse_on_unsaved_object. I have not found any other problems which should prevent relations between unsaved objects, especially because they can be created anyway.

Attachments (1)

one-to-one.patch (2.1 KB ) - added by Vlastimil Zíma 10 years ago.

Download all attachments as: .zip

Change History (5)

by Vlastimil Zíma, 10 years ago

Attachment: one-to-one.patch added

comment:1 by Aymeric Augustin, 10 years ago

Has patch: unset
Summary: Enable one-to-one reverse cache on unsaved objectsPrevent setting nullable relations on unsaved objects
Triage Stage: UnreviewedAccepted

Please test what happens when your save place and bar and reload them from the database. I suspect you'll get surprising results (after reloading bar from the database, bar.place will be None) and you'll understand that your change is re-creating the trap that #18153 removed.

However, you have a good point about symmetry. bar.place = place should raise an exception, and here it doesn't. It's implemented in the ORM as bar.place_id = place.id. When place isn't saved, it doesn't have an id, so this line does bar.place_id = None. This is allowed because place is declared as null=True. Clearly this behaviour is undesirable and Django should raise an exception instead.

I'm changing the title of the ticket to reflect the actual problem, which is different from your interpretation.

There's another ticket about changing the way relations are implemented in the ORM, but if the interest of resolving this ticket without waiting 5 or 10 years, I suggest simply raising an exception in that case.

comment:2 by Vlastimil Zíma, 10 years ago

In my code I am aware of the tricks needed to save related objects, so I do

place.save()
bar.place_id = place.pk
bar.save()

Is it possible to update cached related objects with primary key, or is it the matter of the other ticket? Also, can you please paste the number of the relations refactoring ticket, I wasn't able to find it.

comment:3 by Aymeric Augustin, 10 years ago

Yes, we really don't want every Django user to memorize such idiosyncrasies.

There was a discussion a few weeks ago about updating automatically the primary key once the object is saved. If you didn't find a ticket, maybe that was on the mailing list — I don't remember exactly.

comment:4 by Tim Graham, 9 years ago

Resolution: duplicate
Status: newclosed

As far as I can tell, this is a duplicate of #10811 which has been fixed in master (1.8).

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