[Django] #34067: django.core.Paginator wrong query slicing
Django
-------------------------------------+-------------------------------------
Reporter: Hristo | Owner: nobody
Trendafilov |
Type: Bug | Status: new
Component: Core | Version: 3.2
(Other) | Keywords:
Severity: Normal | Paginator,slice,queryset
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
A have a ListView defined like so:
{{{
class AllPracticesListView(LoginRequiredMixin, PermissionRequiredMixin,
ListView, ToolBoxFunctions):
template_name = 'practice_list.html'
model = Practice
paginate_by = 6
permission_required = 'customauth.access_practices'
def get_queryset(self):
qs = self.get_all_practices_by_user().order_by(
'company_branch__pk',
'practice_profile__personal_profile__last_name').prefetch_related('practice_profile')
# method < get_all_practices_by_user > is a toolbox method that
returns:
#
Practice.objects.filter(company_branch__owner=self.request.user).order_by('pk')
return qs
}}}
Models are defined like so:
{{{
class Practice(models.Model):
company_branch = models.ForeignKey(
CompanyBranch,
on_delete=models.PROTECT,
related_name='practices',
)
# CompanyBranch model has an owner field which ForeignKey
to the user model
practice_profile = models.ForeignKey(
PracticeProfile,
on_delete=models.PROTECT,
related_name='practices',
)
class PracticeProfile(models.Model):
personal_profile = models.ForeignKey(
PersonalProfile,
on_delete=models.PROTECT,
blank=True,
null=True,
)
# PersonalProfile model has a field called < last_name
>
}}}
Or schematically:
{{{
Practice object relations:
-> CompanyBranch
-> PracticeProfile
-> PersonalProfile /*NULLABLE RELATION/
}}}
When the code is run, the view does not display results correctly.
I did like so to get the WRONG result:
Added a breakpoint on {{{Paginator._get_page}}} return statement
Queryset is passed okay and is available in
{{{Paginator.object_list}}}
But in the args of {{{Paginator._get_page}}} the queryset is totally
different than expected / should be a slice from the first six elements of
{{{Paginator.object_list}}} /
I have tried to add a breakpoint like so
[[Image(https://imgur.com/SdaQUt6)]]
Then I get this result [[Image(https://imgur.com/5zzLNV0)]].
As you can see, PKs 1632, 1624, etc. are missing, objects are
different/marked red and purple/, and are actually removed and never shown
in the view.
If you run slice manually at this point - everything is again fine
[[Image(https://imgur.com/yvn8KMO)]]
This happens only on NULL PersonalProfile objects.
I did like so to get OKAY results:
1. Added a simple breakpoint is added on {{{Paginator.page}}} like so
[[Image(https://imgur.com/d7J5BMZ)]] and that breakpoint is just run then
the result is okay [[Image(https://imgur.com/ILDHAMN)]]
2. changed paginate_by to 10
3. changed {{{order_by}}} in {{{get_queryset}}} to {{{-pk}}}
4. call again the {{{page = paginator.page(page_number)}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34067>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "images.zip" added.
Some of the images
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hristo Trendafilov):
* Attachment "images2.zip" added.
Other images
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
return qs
}}}
}}}
P.S. That is also happening in django 2.2
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
Thanks for the report, however `Paginator` works for me. I'm also not sure
why you're using `_get_page()` which is a private undocumented API. I
don't think you've explained the issue in enough detail to confirm a bug
in Django. Please reopen the ticket if you can debug your issue and
provide details about why and where Django is at fault. If you're having
trouble in debugging, see TicketClosingReasons/UseSupportChannels for ways
to get help.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> P.S. That is also happening in django 2.2
New description:
return qs
}}}
}}}
I have added a breakpoint like so [[Image(https://imgur.com/SdaQUt6)]]. I
do not use the method itself, just stopped the execution to check what
data the `Page` class itself is called with.
Then I get this result [[Image(https://imgur.com/5zzLNV0)]].
As you can see, PKs 1632, 1624, etc. are missing, objects are
different/marked red and purple/, and are actually removed and never shown
in the view.
If you run slice manually at this point - everything is again fine
[[Image(https://imgur.com/yvn8KMO)]]
This happens only on NULL PersonalProfile objects.
I did like so to get OKAY results:
1. Added a simple breakpoint is added on {{{Paginator.page}}} like so
[[Image(https://imgur.com/d7J5BMZ)]] and that breakpoint is just run then
the result is okay [[Image(https://imgur.com/ILDHAMN)]]
2. changed paginate_by to 10
3. changed {{{order_by}}} in {{{get_queryset}}} to {{{-pk}}}
4. call again the {{{page = paginator.page(page_number)}}}
P.S. That is also happening in django 2.2
--
Comment (by Hristo Trendafilov):
Replying to [comment:2 Mariusz Felisiak]:
> Thanks for the report, however `Paginator` works for me. I'm also not
sure why you're using `_get_page()` which is a private undocumented API. I
don't think you've explained the issue in enough detail to confirm a bug
in Django. Please reopen the ticket if you can debug your issue and
provide details about why and where Django is at fault. If you're having
trouble in debugging, see TicketClosingReasons/UseSupportChannels for ways
to get help.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:3>
Django
Old description:
New description:
return qs
}}}
}}}
I have tried to add a breakpoint like so
[[Image(https://imgur.com/SdaQUt6)]]
Then I get this result [[Image(https://imgur.com/5zzLNV0)]].
As you can see, PKs 1632, 1624, etc. are missing, objects are
different/marked red and purple/, and are actually removed and never shown
in the view.
If you run slice manually at this point - everything is again fine
[[Image(https://imgur.com/yvn8KMO)]]
This happens only on NULL PersonalProfile objects.
I did like so to get OKAY results:
1. Added a simple breakpoint is added on {{{Paginator.page}}} like so
[[Image(https://imgur.com/d7J5BMZ)]] and that breakpoint is just run then
the result is okay [[Image(https://imgur.com/ILDHAMN)]]
2. changed paginate_by to 10
3. changed {{{order_by}}} in {{{get_queryset}}} to {{{-pk}}}
4. call again the {{{page = paginator.page(page_number)}}}
P.S. That is also happening in django 2.2
--
Comment (by Hristo Trendafilov):
Replying to [comment:2 Mariusz Felisiak]:
> Thanks for the report, however `Paginator` works for me. I'm also not
sure why you're using `_get_page()` which is a private undocumented API. I
don't think you've explained the issue in enough detail to confirm a bug
in Django. Please reopen the ticket if you can debug your issue and
provide details about why and where Django is at fault. If you're having
trouble in debugging, see TicketClosingReasons/UseSupportChannels for ways
to get help.
I don't use `_get_page()`, I have just added a breakpoint inside it so
that I could trace what is populated inside of it since it is what
generates the `Page` object after all. It is clear that args that `Page`
object is called with are different than expected.
What more details do you need me to provide?
I could debug and replicate that issue every time.
It is clearly visible from the screenshot that I have provided that
slicing gives a different result than expected and is also skipping data
which is happening in a production environment.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
You should explain why Django is at fault. I'm guessing that your
complicated query might not be returning results in a consistent order.
There's nothing Django can do about that.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
return qs
}}}
}}}
It is Django fault for those reasons:
* `order_by` might be complicated but is supported by the framework
* Chained `order_by` is also supported
* queryset slicing is a feature of Django queryset API.
* queryset slicing should act like list and string slicing and namely to
provide you with results `[ from : to ]` indexes provided and not to
change or lose data in between. Or in other words - what is passed in the
`Paginator.object_list` should be just sliced `[ from : to ]`. `Paginator`
should never care for the queryset, what is the ordering of the queryset,
what is excluded and so on - it should just work with it.
* changing `paginate_by` to `10` or greater fixes the problem.
`paginate_by` is a standard and documented Django ListView setting.
* changing `order_by` to some other `order_by` also fixes the problem.
`order_by` should work the same or if any limitations those should be well
documented
* removing `order_by` also fixes the problem.
P.S. That is also happening in Django 2.2
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hristo Trendafilov):
Replying to [comment:6 Hristo Trendafilov]:
Just added that in the description. At the end of the day, it is expected
Paginator just slice whatever is provided, not to change or lose data in
between. Changing `paginate_by` leads to fix and that is a feature of
Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Hristo Trendafilov | Owner: nobody
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage:
Paginator,slice,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
> It is Django fault for those reasons:
>
> * `order_by` might be complicated but is supported by the framework
> * Chained `order_by` is also supported
Yes, but you should be aware what you are using in the `ORDER BY` clause
and that some fields may cause issues, e.g. multi-valued fields (see
[https://docs.djangoproject.com/en/stable/ref/models/querysets/#order-by
notes in docs]). Django cannot decide for you.
> * queryset slicing is a feature of Django queryset API.
> * queryset slicing should act like list and string slicing and namely to
provide you with results `[ from : to ]` indexes provided and not to
change or lose data in between. Or in other words - what is passed in the
`Paginator.object_list` should be just sliced `[ from : to ]`. `Paginator`
should never care for the queryset, what is the ordering of the queryset,
what is excluded and so on - it should just work with it.
This would require fetching all results and slicing on the Python side not
in the database which is very ineffective and would cause many performance
issues.
> * changing `paginate_by` to `10` or greater fixes the problem.
`paginate_by` is a standard and documented Django ListView setting.
> * changing `order_by` to some other `order_by` also fixes the problem.
`order_by` should work the same or if any limitations those should be well
documented
Limitations are mostly documented, but we cannot document all caveats.
> P.S. That is also happening in Django 2.2
Django 2.2 is not supported anymore, and Django 3.2 is in extended support
so it receives only security patches.
I understand that you spend a lot of time trying to debug your issue, but
we don't have your app, your data, and your database, so we're unable to
help. Also, Trac is not a support channel.
I'd like to highly recommend using one of
[https://code.djangoproject.com/wiki/TicketClosingReasons/UseSupportChannels
support channel] where folks will help you. If you'll still be convinced
that Django is at fault, then please attach a sample minimal project that
reproduces the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/34067#comment:9>