id_list argument to in_bulk() does not account for composite primary keys when batching

[Jacob Walls]

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 ​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):

tests/composite_pk/tests.py

@@ -147 +147 @@
         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.

[Jacob Walls]

​PR

[Natalia Bidart]

Hello Jacob, thank you for the report and for providing a test! I'm having a bit of an issue when running the test, it seems to hang and never finishes. I've waited over 15 minutes. I have printed the values used in the tests and I have:

connection.features.max_query_params=250000
num_requiring_batching=125001

while high, these do not seem impossible. Specifically, the line that hangs is:

Comment.objects.in_bulk(id_list=id_list)

I would like to understand more about what/why this is happening before accepting. Is this failing for you on main without your proposed fix? Or is it hanging as well?

[Jacob Walls]

I left a ​note here that I believe the slowness is simply due to the unfixed #36380 (and I added mocking in order to avoid creating such a large batch either way). The draft test finishes for me when I apply the fix for #36380 -- sorry I didn't pull these pieces together into the ticket description.

[Jacob Walls]

(And yes, the draft test provided with the ticket is hanging on main for me: when CTRL-C'ing out of the hang, you can see from the trace that the issue is #36380.)

The draft test on the ticket fails on 5.2.1 if you make adjust it to run:

• provide user=self.user in the Comment(... instantiation
• on SQLite: adjust num_requiring_batching to circa 999 to see OperationalError because below Django 6.0 max_query_params is too protective (#36143)

[Natalia Bidart]

Replying to Jacob Walls:

I left a ​note here that I believe the slowness is simply due to the unfixed #36380 (and I added mocking in order to avoid creating such a large batch either way). The draft test finishes for me when I apply the fix for #36380 -- sorry I didn't pull these pieces together into the ticket description.

Thank you, I missed this. When running the given test with the fixes for #36380, I also get the django.db.utils.OperationalError: too many SQL variables.

[Sarah Boyce <42296566+sarahboyce@…>]

In 26313bc:

Fixed #36416 -- Made QuerySet.in_bulk() account for composite pks in id_list.

[Sarah Boyce <42296566+sarahboyce@…>]

In 2bf4c5b9:

[5.2.x] Fixed #36416 -- Made QuerySet.in_bulk() account for composite pks in id_list.

Backport of 26313bc21932d0d3af278ab387549d63b1f64575 from main.

[nessita <124304+nessita@…>]

In a68e8565:

Refs #34378, #36143, #36416 -- Fixed isolation of LookupTests.test_in_bulk_preserve_ordering_with_batch_size().

max_query_params is a property, so it must be patched on the class.