Opened 21 months ago
Closed 21 months ago
#34378 closed Bug (fixed)
Using in_bulk() with id_list and order_by()
Reported by: | Ekaterina Vakhrusheva | Owned by: | Ekaterina Vakhrusheva |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.1 |
Severity: | Normal | Keywords: | ib_bulk |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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.
Change History (12)
follow-up: 3 comment:1 by , 21 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 21 months ago
Description: | modified (diff) |
---|---|
Triage Stage: | Accepted → Unreviewed |
comment:3 by , 21 months ago
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()
comment:4 by , 21 months ago
Triage Stage: | Unreviewed → Accepted |
---|
I think you misunderstood my reply: "it's not useless". I accepted the ticket.
comment:5 by , 21 months ago
Ekaterina, would you like to prepare a patch? (a regression test is required.)
comment:6 by , 21 months ago
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
comment:7 by , 21 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 21 months ago
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
).
comment:10 by , 21 months ago
Has patch: | set |
---|
comment:11 by , 21 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Ordering was removed in #15116, however we shouldn’t clear it anymore because dictionaries preserve the order, so it's not useless.