#36116: prefetch_related() selects too many objects from a related table with a
composite primary key and hidden related name
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Noticed a "fixme"
[https://github.com/django/django/blob/c28f821c9067050ba0d099349a4dfea2b29faf99/django/db/models/fields/related_descriptors.py#L170-L180
comment] in the code that pointed me here. With a hidden related name,
e.g. `comments+`, we hit a branch that expects primary keys to consist of
one column only. When a composite primary key violates that assumption,
more objects than necessary will be fetched.
With this test:
{{{#!diff
diff --git a/tests/composite_pk/models/__init__.py
b/tests/composite_pk/models/__init__.py
index 5996ae33b0..40750d91ae 100644
--- a/tests/composite_pk/models/__init__.py
+++ b/tests/composite_pk/models/__init__.py
@@ -1,7 +1,16 @@
-from .tenant import Comment, Post, Tenant, TimeStamped, Token, User
+from .tenant import (
+ Comment,
+ CommentWithHiddenUserField,
+ Post,
+ Tenant,
+ TimeStamped,
+ Token,
+ User,
+)
__all__ = [
"Comment",
+ "CommentWithHiddenUserField",
"Post",
"Tenant",
"TimeStamped",
diff --git a/tests/composite_pk/models/tenant.py
b/tests/composite_pk/models/tenant.py
index 6286ed2354..66fb371d3a 100644
--- a/tests/composite_pk/models/tenant.py
+++ b/tests/composite_pk/models/tenant.py
@@ -46,6 +46,25 @@ class Comment(models.Model):
text = models.TextField(default="", blank=True)
+class CommentWithHiddenUserField(models.Model):
+ pk = models.CompositePrimaryKey("tenant", "id")
+ tenant = models.ForeignKey(
+ Tenant,
+ on_delete=models.CASCADE,
+ related_name="comments+",
+ )
+ id = models.SmallIntegerField(unique=True, db_column="comment_id")
+ user_id = models.SmallIntegerField()
+ user = models.ForeignObject(
+ User,
+ on_delete=models.CASCADE,
+ from_fields=("tenant_id", "user_id"),
+ to_fields=("tenant_id", "id"),
+ related_name="comments+",
+ )
+ text = models.TextField(default="", blank=True)
+
+
class Post(models.Model):
pk = models.CompositePrimaryKey("tenant_id", "id")
tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE,
default=1)
diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py
index 6b09480fb0..29da1be47b 100644
--- a/tests/composite_pk/tests.py
+++ b/tests/composite_pk/tests.py
@@ -16,8 +16,9 @@ from django.db import IntegrityError, connection
from django.db.models import CompositePrimaryKey
from django.forms import modelform_factory
from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
-from .models import Comment, Post, Tenant, TimeStamped, User
+from .models import Comment, CommentWithHiddenUserField, Post, Tenant,
TimeStamped, User
class CommentForm(forms.ModelForm):
@@ -38,6 +39,9 @@ class CompositePKTests(TestCase):
email="[email protected]",
)
cls.comment = Comment.objects.create(tenant=cls.tenant, id=1,
user=cls.user)
+ cls.comment_by_hidden_user =
CommentWithHiddenUserField.objects.create(
+ tenant=cls.tenant, id=1, user=cls.user
+ )
@staticmethod
def get_primary_key_columns(table):
@@ -157,6 +161,13 @@ class CompositePKTests(TestCase):
result = list(Comment.objects.iterator())
self.assertEqual(result, [self.comment])
+ def test_prefetch_related_hidden_related_name(self):
+ with CaptureQueriesContext(connection) as queries:
+
CommentWithHiddenUserField.objects.prefetch_related("user").get(
+ pk=self.comment_by_hidden_user.pk
+ )
+ self.assertIn('"comment_id" = 1', queries[1]["sql"])
+
def test_query(self):
users = User.objects.values_list("pk").order_by("pk")
self.assertNotIn('AS "pk"', str(users.query))
}}}
The `WHERE` is too permissive -- would expect both primary key columns to
be checked, i.e. also "comment_id".
{{{#!py
======================================================================
FAIL: test_prefetch_related_hidden_related_name
(composite_pk.tests.CompositePKTests.test_prefetch_related_hidden_related_name)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/source/django/tests/composite_pk/tests.py", line 169, in
test_prefetch_related_hidden_related_name
self.assertIn('"comment_id" = 1', queries[1]["sql"])
AssertionError: '"comment_id" = 1' not found in 'SELECT
"composite_pk_user"."tenant_id", "composite_pk_user"."id",
"composite_pk_user"."email" FROM "composite_pk_user" WHERE
"composite_pk_user"."tenant_id" IN (1)'
----------------------------------------------------------------------
Ran 1 test in 0.006s
FAILED (failures=1)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36116>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701948100490f-751f5a98-5679-4a65-96b2-5e6f48180497-000000%40eu-central-1.amazonses.com.
