Opened 13 months ago

Closed 8 months ago

#22726 closed Bug (duplicate)

Prevent setting nullable relations on unsaved objects

Reported by: vzima 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 vzima 13 months ago.

Download all attachments as: .zip

Change History (5)

Changed 13 months ago by vzima

comment:1 Changed 13 months ago by aaugustin

  • Has patch unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Enable one-to-one reverse cache on unsaved objects to Prevent setting nullable relations on unsaved objects
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 13 months ago by vzima

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 Changed 13 months ago by aaugustin

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 Changed 8 months ago by timgraham

  • Resolution set to duplicate
  • Status changed from new to closed

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