#36416: id_list argument to in_bulk() does not account for composite primary
keys
when batching
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Jacob Walls:
Old description:
> The untested `id_list` argument to `in_bulk()` divides large lists into
> batches, but similar to #36118, it did not account for composite primary
> keys, leading to errors like this on SQLite:
>
> {{{
> django.db.utils.OperationalError: too many SQL variables
> }}}
>
> #36118 mentioned that other uses like this remained to be audited.
>
> Failing test (I may adjust this in the PR to run faster by mocking a
> lower query param limit, but this shows the OperationalError):
> {{{#!diff
> diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py
> index 91cbee0635..ba290a796f 100644
> --- a/tests/composite_pk/tests.py
> +++ b/tests/composite_pk/tests.py
> @@ -147,6 +147,18 @@ class CompositePKTests(TestCase):
> result = Comment.objects.in_bulk([self.comment.pk])
> self.assertEqual(result, {self.comment.pk: self.comment})
>
> + def test_in_bulk_batching(self):
> + num_requiring_batching = (connection.features.max_query_params
> // 2) + 1
> + comments = [
> + Comment(id=i, tenant=self.tenant)
> + for i in range(2, num_requiring_batching + 1)
> + ]
> + Comment.objects.bulk_create(comments)
> + id_list = list(Comment.objects.values_list("pk", flat=True))
> + with self.assertNumQueries(2):
> + comment_dict = Comment.objects.in_bulk(id_list=id_list)
> + self.assertQuerySetEqual(comment_dict, id_list)
> +
> def test_iterator(self):
> """
> Test the .iterator() method of composite_pk models.
> }}}
New description:
The `id_list` argument to `in_bulk()` divides large lists into batches,
but similar to #36118, it did not account for composite primary keys,
leading to errors like this on SQLite:
{{{
django.db.utils.OperationalError: too many SQL variables
}}}
#36118 mentioned that other uses like this remained to be audited.
The id_list argument is tested, but the batching was
[https://djangoci.com/view/%C2%ADCoverage/job/django-
coverage/HTML_20Coverage_20Report/z_d81526da7cfdb7e4_query_py.html#t1155
not covered].
Failing test (I may adjust this in the PR to run faster by mocking a lower
query param limit, but this shows the OperationalError):
{{{#!diff
diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py
index 91cbee0635..ba290a796f 100644
--- a/tests/composite_pk/tests.py
+++ b/tests/composite_pk/tests.py
@@ -147,6 +147,18 @@ class CompositePKTests(TestCase):
result = Comment.objects.in_bulk([self.comment.pk])
self.assertEqual(result, {self.comment.pk: self.comment})
+ def test_in_bulk_batching(self):
+ num_requiring_batching = (connection.features.max_query_params //
2) + 1
+ comments = [
+ Comment(id=i, tenant=self.tenant)
+ for i in range(2, num_requiring_batching + 1)
+ ]
+ Comment.objects.bulk_create(comments)
+ id_list = list(Comment.objects.values_list("pk", flat=True))
+ with self.assertNumQueries(2):
+ comment_dict = Comment.objects.in_bulk(id_list=id_list)
+ self.assertQuerySetEqual(comment_dict, id_list)
+
def test_iterator(self):
"""
Test the .iterator() method of composite_pk models.
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/36416#comment:2>
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/0107019703877a8c-0046fee9-6627-4392-b833-1ae1ef6f140c-000000%40eu-central-1.amazonses.com.
