Opened 10 years ago

Closed 7 years ago

#3665 closed (duplicate)

select_related() should generate LEFT JOINs for foreign keys with null=True

Reported by: semenov@… Owned by: Malcolm Tredinnick
Component: Core (Other) Version: 1.1
Severity: Keywords: orm select_related left join
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Consider a (simplified) real-life example:

class Ticket(models.Model): 
    is_resolved = models.BooleanField() 
    resolved_by = models.ForeignKey(User, null=True)


...a view which is showing all resolved tickets:

def resolved_tickets(request): 
    tickets = Ticket.objects.filter(is_resolved=True).select_related() 
    return HttpResponse(......) 


...and a template which is doing something like:

{% for t in tickets %} 
    {{ t }} resolved by {{ t.user }} 
{% endfor %} 

(I believe this is a very common use case; I've run into it several times while developing my first application!)

Since user field has null=True, select_related() doesn't do its job and the code generates N+1 queries. However, from SQL point of view, the whole bunch of queries can be replaced with a single simple LEFT JOIN query, and I would expect Django ORM to allow this.

I realize that under some circumstances, LEFT JOINs can be costly. I also realize that LEFT JOINs can hardly traverse through several nesting levels.
That's why, I would like to suggest the following (unobtrusive and backwards-compatible!) behaviour:

  1. If select_related() called with depth argument (as of [4645]), the last level of recursion uses LEFT JOINs instead of INNER JOINs for foreign keys.
  1. If select_related() called with fields argument (once the fields suggestion from #3275 is committed) and the list of fields contains nullable foreign key(s), these particular key(s) are joined using LEFT JOIN, and the recursion stops for them.

As far as I understand, this will neither break any old code, nor make any perfomance impact; while being very useful in terms of database load optimization.

Change History (7)

comment:1 Changed 10 years ago by Simon G. <dev@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Summary: select_related() to generate LEFT JOINs for foreign keys with null=Trueselect_related() should generate LEFT JOINs for foreign keys with null=True
Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 10 years ago by Ilya Semenov <semenov@…>

Thinking more about the issue, I came up with the more generic proposal:

select_related() should always use LEFT JOINs for all nullable foreign keys. The recursion should not proceed to these fields.

Two changes that I mentioned in the ticket description will therefore be just special cases of this generic rule.

comment:3 Changed 10 years ago by Malcolm Tredinnick

Owner: changed from Adrian Holovaty to Malcolm Tredinnick
Triage Stage: Design decision neededAccepted

This will be fixed in the QuerySet rewrite that is happening at the moment. We can definitely cut down on the number of queries there. Your logic, by the way, is correct -- always use left outer joins in this case.

comment:4 Changed 10 years ago by mir@…

mtredinnick: maybe this should be a duplicate of #3592?

comment:5 Changed 10 years ago by James Bennett

Resolution: duplicate
Status: newclosed

Certainly looks to be. Closing as duplicate of #3592.

comment:6 Changed 7 years ago by retrogradeorbit

Resolution: duplicate
Status: closedreopened
Version: SVN1.1

This ticket is still not fixed as it was never really a duplicate of #3592. The behavior explained in this ticket still persists in the most recent versions of Django and this ticket describes the problem exactly. In fact the best description of the problem from the ticket is:

"select_related() should always use LEFT JOINs for all nullable foreign keys. The recursion should not proceed to these fields."

comment:7 Changed 7 years ago by Alex Gaynor

Resolution: duplicate
Status: reopenedclosed

This ticket was closed over 2 years ago, please do not reopen it. If you think there is a new bug please file a new bug.

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