QuerySet.select_related() with multiple filtered relations to the OneToOneField sets the last one.

[zhu]

​https://github.com/django/django/commit/e1ae2b00504ba30481285b2bd767d1ad561bf4be
​https://github.com/django/django/blob/0bd2c0c9015b53c41394a1c0989afbfd94dc2830/django/db/models/sql/compiler.py#L1290
should use partial, just like the remote_setter

[Mariusz Felisiak]

Replying to zhu:

should use partial, just like the remote_setter

Why? it doesn't use a name values from the loop. I don't think you've explained the issue.

[Mariusz Felisiak]

Replying to Mariusz Felisiak:

Why? it doesn't use a name values from the loop. I don't think you've explained the issue.

Ahh, OK, it uses f. Can you provide a regression test?

[zhu]

Replying to Mariusz Felisiak:

Replying to Mariusz Felisiak:

Why? it doesn't use a name values from the loop. I don't think you've explained the issue.

Ahh, OK, it uses f. Can you provide a regression test?

  # models.py
from django.db import models

class Pool(models.Model):
    name = models.CharField(max_length=30)


class AnotherPoolStyle(models.Model):
    name = models.CharField(max_length=30)
    pool1 = models.OneToOneField(Pool, models.CASCADE,
            related_name='style_from_pool1')
    pool2 = models.OneToOneField(Pool, models.CASCADE,
            related_name='style_from_pool2')

# tests.py
from django.test import TestCase

from django.db.models import FilteredRelation

class Tests(TestCase):
    @classmethod
    def setUpTestData(cls):
        cls.p1 = Pool.objects.create(name="pool1")
        cls.p2 = Pool.objects.create(name="pool2")
        cls.ap = AnotherPoolStyle.objects.create(
                name="Another Pool Style",
                pool1=cls.p1,
                pool2=cls.p2,
                )

    def test_filtered_relation_select_related(self):
        with self.assertNumQueries(1):
            ps = list(AnotherPoolStyle.objects.annotate(
                p1=FilteredRelation('pool1'),
                p2=FilteredRelation('pool2'),
                ).select_related('p1', 'p2'))
            self.assertIs(ps[0], ps[0].p1.style_from_pool1)  # failed
            self.assertIs(ps[0], ps[0].p2.style_from_pool2)

 

[Mariusz Felisiak]

Thanks! Would you like to prepare a patch?

[zhu]

Replying to Mariusz Felisiak:

Thanks! Would you like to prepare a patch?

diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index 2b66ab12b4..831ef79269 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1261,7 +1261,7 @@ class SQLCompiler:
                 )
                 get_related_klass_infos(klass_info, next_klass_infos)

-            def local_setter(obj, from_obj):
+            def local_setter(f, obj, from_obj):
                 # Set a reverse fk object when relation is non-empty.
                 if from_obj:
                     f.remote_field.set_cached_value(from_obj, obj)
@@ -1287,7 +1287,7 @@ class SQLCompiler:
                         "model": model,
                         "field": f,
                         "reverse": True,
-                        "local_setter": local_setter,
+                        "local_setter": partial(local_setter, f),
                         "remote_setter": partial(remote_setter, name),
                         "from_parent": from_parent,
                     }

[Mariusz Felisiak]

Thanks, can you prepare PR (with patch and tests) and ​submit it via GitHub? (docs changes are not required.)

[zhu]

Replying to Mariusz Felisiak:

Thanks, can you prepare PR (with patch and tests) and ​submit it via GitHub? (docs changes are not required.)

​https://github.com/django/django/pull/16398 done.

[GitHub <noreply@…>]

In e07e8358:

Refs #34226 -- Renamed local field variables in SQLCompiler.get_related_selections() to avoid redefinition.

[Mariusz Felisiak <felisiak.mariusz@…>]

In bbeeb45:

Fixed #34226 -- Fixed QuerySet.select_related() with multiple FilteredRelations to the OneToOneField.