Code

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9554 closed (wontfix)

admin app's change list usage of select related - maximum depth

Reported by: gert Owned by: nobody
Component: Contrib apps Version: 1.0
Severity: Keywords: select_related list_display list_select_related
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In applications with deep object relationships "select related" often results in huge queries that can hit the database very hard. I.e. we have an accounting module that immediately adds 10 levels of related objects to your average "client" object.

I think don't think it makes sense the select related for more than 2 or 3 relationships deep. My suggestion is to add a "maximum depth" parameter to select related that defaults to something sensible like 3 or at least make it configurable.

Attachments (0)

Change History (9)

comment:1 Changed 5 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

What about a depth argument like implemented and documented at http://docs.djangoproject.com/en/dev/ref/models/querysets/#id4 ?
(granted, perhaps it should be included in the signature in the section title)

comment:2 Changed 5 years ago by gert

  • Component changed from Database layer (models, ORM) to Contrib apps
  • Resolution invalid deleted
  • Status changed from closed to reopened

I think maybe a better place to address this issue is in the admin application by just adding a depth=3 parameter to the two select_related instructions in admin/views/main.py

def get_query_set(self):
    ...

        # Use select_related() if one of the list_display options is a field
        # with a relationship.
        if self.list_select_related:
            qs = qs.select_related(depth=3)
        else:
            for field_name in self.list_display:
                try:
                    f = self.lookup_opts.get_field(field_name)
                except models.FieldDoesNotExist:
                    pass
                else:
                    if isinstance(f.rel, models.ManyToOneRel):
                        qs = qs.select_related(depth=3)
                        break


comment:3 Changed 5 years ago by ramiro

So, you were talking specifically about the admin contrib app?.

comment:4 Changed 5 years ago by mtredinnick

This doesn't sound like a clear problem description to me yet. Select_related already has a default maximum depth of 5, which already meets the definition of "sensible" for many cases. I don't see any great need to micromanage at that point by saying "perhaps 2 or 3 is better" -- we just pick a value and stick with it and 5 is the current value. We could be here all day debating 2 or 3 or 5 or 7.

So the real issue is why this particular problem report is not respecting the maximum depth of 5. If it is, then this becomes a proposal to change 5 to 3 and ... meh.

comment:5 Changed 5 years ago by gert

ramiro: yes, I only picked it up in the admin contrib app. At that stage I was unaware of the depth=X parameter. I think it adequately addresses the issue for custom applications, also I don't think internally select_related needs to drop the 5 down to 3.

BUT, for the admin application I think a tweak is required. Arguments for explicitly lowering select_related for admin to say 3 :) are;

1) Whatever value you take must hit the sweat spot for 90% of situations otherwise it will slow down rather than increase overall application speed. I can not even think of an example where I want to go down 5 levels. (object.level1.level2.level3.level4.level5)

2) Remember that the effect on the database increases exponentially, e.g.

3 levels 2 references per object => 32 = 9 DB tables

5 levels 2 references per object => 52 = 25 DB tables

5 levels 3 references per object => 53 = 125 DB tables

comment:6 Changed 5 years ago by ramiro

  • Keywords select_related list_display list_select_related added
  • Summary changed from select related - maximum depth to admin app's change list usage of select related - maximum depth

comment:7 Changed 5 years ago by ramiro

#9849 (closed as a duplicate of this ticket) proposed being able to control the select related queries by being able to specify which relationships to follow.

comment:8 Changed 5 years ago by jacob

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

Yeah, let's not micromanage the admin like this. You can just override queryset if you need advanced usage.

comment:9 Changed 5 years ago by ramiro

  • Keywords select related removed

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.