Opened 19 months ago

Closed 17 months ago

Last modified 17 months ago

#21240 closed Bug (fixed)

Breaking change regarding select_related+OneToOneField missing from release notes

Reported by: marcin@… Owned by: nobody
Component: Documentation Version: 1.5
Severity: Normal Keywords: breaking change
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django 1.5 made changes to the way select_related interacts with OneToOneField reverse relations: https://code.djangoproject.com/ticket/13839

This is a breaking change and was not included in the release notes.

The behaviour of this interaction was previously undocumented (and had an open bug), but since it is core ORM functionality, I don't think you can fault people for relying on it.

Please document it in 1.5's release notes, preferably with its own paragraph.

Change History (9)

comment:1 Changed 19 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Could you add an example of what breaks and how? This should include example models, query and how the query behaves in 1.4 compared to 1.5.

comment:2 Changed 19 months ago by marcin@…

Models:

class Nut (models.Model):
    pass

class Bolt (models.Model):
    nut = models.OneToOneField (Nut)

Relevant tests:

class FindingOrphanedNutsTestcase (TestCase):
    
    def test_old_behaviour__will_fail_on_1_5_and_later (self): # fails by letting DoesNotExist escape
        nut = Nut.objects.create ()
        nuts = [nut for nut in Nut.objects.all().select_related("bolt") if nut.bolt is None]
        self.assertListEqual (nuts, [nut])
    
    def test_new_behaviour__will_fail_on_1_4_and_earlier (self): # fails by producing an empty list
        nut = Nut.objects.create ()
        
        nuts = []
        for nut in Nut.objects.all().select_related("bolt"):
            try:
                nut.bolt
            except Bolt.DoesNotExist:
                nuts.append (nut)
        
        self.assertListEqual (nuts, [nut])

Now of course you could write that to accommodate both None and DoesNotExist (or just with a .filter), but that's not the point, it's the fact that old code for many years and releases had to handle None, but didn't have to handle DoesNotExist. And with 1.5 it suddenly has to handle DoesNotExist and can disregard None.

comment:3 Changed 19 months ago by marcin@…

I just realized I made a serious faux pas in the testcase when pasting code around (inadvertently re-using the local "nut"), it still works fine though. If someone could edit and change the loop variable name, that'd be lovely, wouldn't want to clutter the ticket with another half-a-screen of nearly identical code.

comment:4 Changed 19 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

A quick mention in backwards incompatible changes seems OK to me. Miscellaneous is probably the right place.

comment:5 Changed 17 months ago by timo

marcin, could you propose text for this that you would have found helpful? I'm not sure how to describe it.

comment:6 Changed 17 months ago by marcin@…

How about "Accessing reverse one-to-one relations fetched via select_related now raises DoesNotExist instead of returning None."

comment:7 Changed 17 months ago by Tim Graham <timograham@…>

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

In ba63b9895b478c0093e61c288df0ff053ad726a8:

Fixed #21240 -- Added 1.5 release note for OneToOneField/select_related change.

Thanks marcin at sokrates.pl.

comment:8 Changed 17 months ago by Tim Graham <timograham@…>

In 2b03d53438e7d54f754c58d8f148254a7a73ef5c:

[1.6.x] Fixed #21240 -- Added 1.5 release note for OneToOneField/select_related change.

Thanks marcin at sokrates.pl.

Backport of ba63b9895b from master

comment:9 Changed 17 months ago by Tim Graham <timograham@…>

In 402967f49c6ee5769c47e2df4699a0d5bd588c52:

[1.5.x] Fixed #21240 -- Added 1.5 release note for OneToOneField/select_related change.

Thanks marcin at sokrates.pl.

Backport of ba63b9895b from master

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