[Django] #36342: Slicing a QuerySet with a result cache results in a list?

[Django]

#36342: Slicing a QuerySet with a result cache results in a list?
-------------------------------------+-------------------------------------
Reporter: Willem Van Onsem | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
As
[https://github.com/django/django/blob/adf2991d32c24f8b2e549a25a7eda52f317a91a6/django/db/models/query.py#L401-L436
per code] if you slice a QuerySet with a result cache, we return the
sliced result cache, this thus means that for a `QuerySet`:

{{{
from django.contrib.auth.model import User

qs = User.objects.all()
bool(qs) # enable/disable

qs[:3].values('id')
}}}

will raise an error because `qs[:3]` returns a list, whereas if we comment
out the `bool`, it will still work.

This is done probably because of performance reasons: if we have a
`QuerySet`, and we already know the results, we can just work with these
results.

But I'm wondering if we "can have the cake and eat it too". We could for
example create a sliced copy of the queryset, and populate the result
cache of the queryset. Something along the lines of:

{{{
def __getitem__(self, k):
"""Retrieve an item or slice from the set of results."""
if not isinstance(k, (int, slice)):
raise TypeError(
"QuerySet indices must be integers or slices, not %s."
% type(k).__name__
)
if (isinstance(k, int) and k < 0) or (
isinstance(k, slice)
and (
(k.start is not None and k.start < 0)
or (k.stop is not None and k.stop < 0)
)
):
raise ValueError("Negative indexing is not supported.")

# remove below
# if self._result_cache is not None:
# return self._result_cache[k]

if isinstance(k, slice):
qs = self._chain()
if k.start is not None:
start = int(k.start)
else:
start = None
if k.stop is not None:
stop = int(k.stop)
else:
stop = None
qs.query.set_limits(start, stop)
if self._result_cache is not None:
# populate the QuerySet
qs._result_cache = self._result_cache[k]
return list(qs)[:: k.step] if k.step else qs
}}}

this thus means that, (a) unless we use a step, we always get a `QuerySet`
for slicing (since it is not always known in advance *if* the `QuerySet`
has a result cache, that can be the result of complicated code flows); and
(b) if the result cache was present, the queryset we generate will not
have to fetch the data, if we don't make more `QuerySet` calls.
--
Ticket URL: <https://code.djangoproject.com/ticket/36342>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36342: Slicing a QuerySet with a result cache results in a list?
-------------------------------------+-------------------------------------

Reporter: Willem Van Onsem | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* resolution: => invalid
* status: new => closed

Comment:

Thank you Willem Van Onsem for taking the time to create this report. I
created a test for this:

{{{#!diff
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index 38b0a5ddfa..10c2b821fc 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -2971,6 +2971,13 @@ class WeirdQuerysetSlicingTests(TestCase):
self.assertQuerySetEqual(Article.objects.values()[n:n], [])
self.assertQuerySetEqual(Article.objects.values_list()[n:n],
[])

+ def test_slice_after_result_cached(self):
+ qs = Article.objects.all().order_by("id")
+ # self.assertIs(bool(qs), True)
+ expected = Article.objects.filter(id__lt=4).order_by("id")
+ self.assertQuerySetEqual(qs[:3], expected)
+ self.assertQuerySetEqual(qs[:3].values("id"),
Article.objects.filter(id__lt=4).values("id"))
+

class EscapingTests(TestCase):
def test_ticket_7302(self):
}}}

If `self.assertIs(bool(qs), True)` is commented out, the test passes. If
it's not, the test fails with:
{{{
======================================================================
ERROR: test_slice_after_result_cached
(queries.tests.WeirdQuerysetSlicingTests.test_slice_after_result_cached)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/nessita/fellowship/django/tests/queries/tests.py", line
2979, in test_slice_after_result_cached
self.assertQuerySetEqual(qs[:3].values("id"),
Article.objects.filter(id__lt=4).values("id"))
^^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'values'
}}}

I couldn't find a dupe, and I was inclined to accept this until I found
the explicit docs about this. From
https://docs.djangoproject.com/en/5.2/ref/models/querysets/:
5.2
> Slicing. As explained in Limiting QuerySets, a QuerySet can be sliced,
using Python’s array-slicing syntax. Slicing an unevaluated QuerySet
usually returns another unevaluated QuerySet, but Django will execute the
database query if you use the “step” parameter of slice syntax, and will
return a list. Slicing a QuerySet that has been evaluated also returns a
list.
> Also note that even though slicing an unevaluated QuerySet returns
another unevaluated QuerySet, modifying it further (e.g., adding more
filters, or modifying ordering) is not allowed, since that does not
translate well into SQL and it would not have a clear meaning either.

Closing as `invalid` given the above.
--
Ticket URL: <https://code.djangoproject.com/ticket/36342#comment:1>

[Django]

#36342: Slicing a QuerySet with a result cache results in a list?
-------------------------------------+-------------------------------------
Reporter: Willem Van Onsem | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Willem Van Onsem):

I think it is indeed not a bug, it is more a feature request, since it
results in unpredictable behavior, since you don't know per se if the
queryset got evaluated. It is more whether we can not, when slicing an
evaluated queryset, return a queryset with cached results. Then the
outcome is not different depending on the *state* of the `QuerySet`.

Slicing with a step is more predictable, since it will *always* return a
list when sliced, whereas the slicing without a step, thus depends on the
state of the `QuerySet` which is harder to determine.
--
Ticket URL: <https://code.djangoproject.com/ticket/36342#comment:2>