Fixed #21410 -- Fixed prefetch_related() for FK relations with related_name='+'. #1898

Closed
wants to merge 1 commit into from

3 participants

@loic
Django member

Regression introduced by commit 9777442.

Thanks to trac username troygrosfield for the report and test case.

@bmispelon bmispelon and 1 other commented on an outdated diff Nov 9, 2013
django/db/models/fields/related.py
@@ -274,7 +274,7 @@ def get_prefetch_queryset(self, instances, queryset=None):
rel_obj_attr = self.field.get_foreign_related_value
instance_attr = self.field.get_local_related_value
instances_dict = dict((instance_attr(inst), inst) for inst in instances)
- query = {'%s__in' % self.field.related_query_name(): instances}
+ query = {'%s__in' % self.field.related_field.name: set([instance_attr(inst)[0] for inst in instances])}
@bmispelon
Django member

Not really important but you don't need the square brackets: set(instance_attr(inst)[0] for inst in instances).

@loic
Django member
loic added a note Nov 9, 2013

Good catch, I added the set() afterwards and forgot to remove the list comprehension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bmispelon bmispelon commented on an outdated diff Nov 9, 2013
django/contrib/contenttypes/generic.py
kwargs['rel'] = GenericRel(
- self, to, related_name=kwargs.pop('related_name', None),
- limit_choices_to=kwargs.pop('limit_choices_to', None),)
+ self, to, limit_choices_to=kwargs.pop('limit_choices_to', None),)
@bmispelon
Django member

Nitpicky, but since we're splitting over two lines, I think we may as well introduce a temporary limit_choices_to variable:

limit_choices_to = kwargs.pop('limit_choices_to', None)
kwargs['rel'] = GenericRel(self, to, limit_choices_to=limit_choices_to)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@troygrosfield

Happy to help. Thanks for the quick turnaround time on this bug! Once approved, any idea when this will get released? This is a showstopper for us moving forward with 1.6.

@troygrosfield

For traceability purposes, adding the link to the trac ticket:

https://code.djangoproject.com/ticket/21410

@loic
Django member

Hi @troygrosfield, could you verify this behaves as expected in your project? If it does, you can mark the ticket as "Ready for checkin" on trac, this will help the ticket to move forward.

Then the procedure goes as follows:

  • Commit on the master branch.
  • Backport to the stable/1.6.x branch.
  • Inclusion in the next scheduled dot release along with other bug fixes.

If this is the only thing stopping you from upgrading, one option will be to point your pip to the github stable/1.6.x branch once the bug has been fixed.

@troygrosfield

Thanks for the info @loic! After updating django to your fix [1], all my tests are passing. I updated the ticket in trac [2].

[1] loic@30683ee
[2] https://code.djangoproject.com/ticket/21410#comment:6

@loic loic Fixed #21410 -- Fixed prefetch_related() for FK relations with relate…
…d_name='+'.

Regression introduced by commit 9777442.

Thanks to trac username troygrosfield for the report and test case.
e6e30aa
@akaariai
Django member

Merged manually.

@akaariai akaariai closed this Nov 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment