#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 django-updates+unsubscr...@googlegroups.com.
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.
