Code

Opened 7 years ago

Closed 4 years ago

#3665 closed (duplicate)

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

Reported by: semenov@… Owned by: mtredinnick
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.

Attachments (0)

Change History (7)

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

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from select_related() to generate LEFT JOINs for foreign keys with null=True to select_related() should generate LEFT JOINs for foreign keys with null=True
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 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 7 years ago by mtredinnick

  • Owner changed from adrian to mtredinnick
  • Triage Stage changed from Design decision needed to Accepted

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 7 years ago by mir@…

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

comment:5 Changed 7 years ago by ubernostrum

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

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

comment:6 Changed 4 years ago by retrogradeorbit

  • Resolution duplicate deleted
  • Status changed from closed to reopened
  • Version changed from SVN to 1.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 4 years ago by Alex

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

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.