[Django] #35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is sliced

23 views
Skip to first unread message

Django

unread,
Aug 9, 2024, 2:46:47 AM8/9/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.1 | Severity: Normal
Keywords: prefetch slice | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In {{{v5.1}}} the Prefetch operator causes an issue when no
{{{order_by()}}} is specified, and the query contains a slice.

* Must have no order by (either explict {{{order_by()}}} or model Meta
contains no order by)
* Must be sliced - any form works, {{{:10}}}, {{{10:}}}, {{{10:20}}}

Does not crash in {{{v5.0.7}}}. I did not see this mentioned in the
{{{Prefetch}}} documentation, or the changelog.

{{{
short_list = Prefetch(
"products",
queryset=Product.objects.order_by()[:10],
to_attr="short_list",
)
query = Store.objects.prefetch_related(short_list)
print(query) # errors
}}}


**The stack trace:**


{{{
File "python3.12/site-packages/django/db/models/expressions.py", line
1896, in __init__
self.order_by = OrderByList(*self.order_by)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "python3.12/site-packages/django/db/models/expressions.py", line
1414, in __init__
super().__init__(*expressions, **extra)
File "python3.12/site-packages/django/db/models/expressions.py", line
1382, in __init__
raise ValueError(
ValueError: OrderByList requires at least one expression.
}}}

I was able to replicate this in a clean project, against sqlite, with the
following 2 simple models, used in the example above:

{{{
class Store(models.Model):
name = models.CharField(max_length=100)

class Product(models.Model):
store = models.ForeignKey(Store, on_delete=models.CASCADE,
related_name='products')
name = models.CharField(max_length=100)
class Meta:
ordering = ['name']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35665>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 9, 2024, 8:11:58 AM8/9/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch slice | Triage Stage: Accepted
order_by |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* keywords: prefetch slice => prefetch slice order_by
* owner: (none) => Simon Charette
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* status: new => assigned

Comment:

I managed to reproduce and the issue the issue is relatively simple to
address.

It should obviously not crash but just so you know using slicing without
ordering generally makes little sense as the backend is allowed to return
rows in any order.
--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:1>

Django

unread,
Aug 9, 2024, 8:59:48 AM8/9/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch slice | Triage Stage: Accepted
order_by |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:2>

Django

unread,
Aug 11, 2024, 11:05:41 PM8/11/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch slice | Triage Stage: Accepted
order_by |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Andrew):

Replying to [comment:2 Simon Charette]:

Yes, it returns them in database order, which is acceptable in this case.

This **actually** came up due to a Model having no {{{Meta.ordering}}},
which is our standard. All ordering must be explicit at the query level.
--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:3>

Django

unread,
Aug 12, 2024, 8:16:23 AM8/12/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: prefetch slice | Triage Stage: Ready for
order_by | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:4>

Django

unread,
Aug 13, 2024, 5:26:25 AM8/13/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: prefetch slice | Triage Stage: Ready for
order_by | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"602fe961e6834d665f2359087a1272e9f9806b71" 602fe96]:
{{{#!CommitTicketReference repository=""
revision="602fe961e6834d665f2359087a1272e9f9806b71"
Fixed #35665 -- Fixed a crash when passing an empty order_by to Window.

This also caused un-ordered sliced prefetches to crash as they rely on
Window.

Regression in e16d0c176e9b89628cdec5e58c418378c4a2436a that made
OrderByList
piggy-back ExpressionList without porting the empty handling that the
latter
provided.

Supporting explicit empty ordering on Window functions and slicing is
arguably
a foot-gun design due to how backends will return undeterministic results
but
this is a problem that requires a larger discussion.

Refs #35064.

Thanks Andrew Backer for the report and Mariusz for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:5>

Django

unread,
Aug 13, 2024, 5:28:38 AM8/13/24
to django-...@googlegroups.com
#35665: Prefetch() fails with "OrderByList" when queryset has no order_by and is
sliced
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: prefetch slice | Triage Stage: Ready for
order_by | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"df236b0bcbbf1f54dfe6acc7761cd81b76ebf2cc" df236b0]:
{{{#!CommitTicketReference repository=""
revision="df236b0bcbbf1f54dfe6acc7761cd81b76ebf2cc"
[5.1.x] Fixed #35665 -- Fixed a crash when passing an empty order_by to
Window.

This also caused un-ordered sliced prefetches to crash as they rely on
Window.

Regression in e16d0c176e9b89628cdec5e58c418378c4a2436a that made
OrderByList
piggy-back ExpressionList without porting the empty handling that the
latter
provided.

Supporting explicit empty ordering on Window functions and slicing is
arguably
a foot-gun design due to how backends will return undeterministic results
but
this is a problem that requires a larger discussion.

Refs #35064.

Thanks Andrew Backer for the report and Mariusz for the review.

Backport of 602fe961e6834d665f2359087a1272e9f9806b71 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35665#comment:6>
Reply all
Reply to author
Forward
0 new messages