Using in_bulk() with id_list and order_by()

[Ekaterina Vakhrusheva]

If you call in_bulk() together with the order_by(“field_1”, “field_2”).distinct(“field_1”) and specifying field_name=“field_1”, then when the result is received, the sorting order of the elements passed to order_by() is preserved. Code example:

locations = (
    UserLocation.objects.order_by("user_id", "-date_create")
    .distinct("user_id")
    .in_bulk(field_name="user_id")
)
print(locations)

>>>{2693: <UserLocation: 2023-03-01 15:17>}

However, if you specify an additional id_list argument in the in_bulk method, the sorting specified earlier in the request disappears, and as a result, another instance of the model is returned. Code example:

locations = (
    UserLocation.objects.order_by("user_id", "-date_create")
    .distinct("user_id")
    .in_bulk([2693], field_name="user_id")
)
print(locations)

>>>{2693: <UserLocation: 2023-03-01 14:27>}

A user with id 2693 has several locations that differ in the date_create. The most recent is with a time of 15:17. When sorting queryset by “-date_create” I want to get the most recent record. Therefore, the sorting is reset when passing the id_list parameter.

In my opinion, the sorting from queryset was removed before it became possible to pass different field_name to the in_bulk() method. Apparently, it was not taken into account that when sorting is reset, the result obtained when using DISTINCT changes.

​https://forum.djangoproject.com/t/using-in-bulk/19192

[Mariusz Felisiak]

Ordering was removed in #15116, however we shouldn’t clear it anymore because dictionaries preserve the order, so it's not useless.

[Ekaterina Vakhrusheva]

Replying to Mariusz Felisiak:

Ordering was removed in #15116, however we shouldn’t clear it anymore because dictionaries preserve the order, so it's not useless.

i think, this is not useless, not only because dictionaries preserve the order, but also because the order of the elements of the queryset affects the value returned by .distinct()

[Mariusz Felisiak]

I think you misunderstood my reply: "it's not useless". I accepted the ticket.

[Mariusz Felisiak]

Ekaterina, would you like to prepare a patch? (a regression test is required.)

[Ekaterina Vakhrusheva]

Yes, it would be great. But I’ve never done patches before. Is there anything I need to know about the patch creation process other than ​this guide

[Mariusz Felisiak]

​Coding style can also be helpful.

This is a bugfixes so docs changes are not necessary.
We need a fix (it should be enough to remove order_by() from the in_bulk() ​implementation) and regression tests in tests.lookup.tests.LookupTests (please be sure that you covered both branches, with and without batch_size).

[Ekaterina Vakhrusheva]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 55bcbd8d:

Fixed #34378 -- Made QuerySet.in_bulk() not clear odering when id_list is passed.

This reverts 340eaded4e30cf25bcd4e9781d33a617fe9c0f84.

[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.

[Natalia <124304+nessita@…>]

In a4e27c0:

[5.2.x] 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.

Backport of a68e8565cdd4fc3f8b738fc516095dab142b9d65 from main.

[Sarah Boyce <42296566+sarahboyce@…>]

In d3cf24e9:

Refs #36430, #36416, #34378 -- Simplified batch size calculation in QuerySet.in_bulk().