Skip to content
/ django Public

Fixes #16855#124

Closed
oinopion wants to merge 1 commit intodjango:masterfrom
oinopion:t16855
Closed

Fixes #16855#124
oinopion wants to merge 1 commit intodjango:masterfrom
oinopion:t16855

Conversation

@oinopion
Copy link
Copy Markdown
Contributor

@oinopion oinopion commented Jun 7, 2012

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query.select_related has numerous states: it can be False (for no select_related), it can be True (which seems to indicate that depth based select_related should be used) or it can be a dict (for user-selected restriction). I think here field_dict could end up being True here.

In the long run it would be better to split the query.select_related to two variables: one would be query.select_related = True/False, another would ne query.select_related_restriction = None or dict_of_asked_selections.

For the time being maybe
field_dict = isinstance(self.select_related, dict) and self.select_related or {}
would do? (naturally, not tested).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the question now is: what to do when select_related is True? Two use cases:

# will work ok
Species.objects.select_related(depth=4).select_related('genus') 

# should get all immediate relations AND the Order
Species.objects.select_related(depth=1).select_related('genus__family__order') 

I seems to me we need this split on select_related and we need to handle it intelligently, ie. 'count' depth of select related field and conditionally add it or not.

@akaariai
Copy link
Copy Markdown
Member

akaariai commented Jun 8, 2012

I am closing this pull request, as the

  Species.objects.select_related(depth=1).select_related('genus__family__order')

case needs some more consideration (more details in the ticket). Please reopen when this issue is dealt with.

@akaariai akaariai closed this Jun 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants