Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#7121 closed (wontfix)

Slicing on QuerySet bypasses result_cache

Reported by: lars@… Owned by: nobody
Component: Uncategorized Version: master
Severity: Keywords: qsrf-cleanup queryset-rf, cache
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by ramiro)

Playing around in the shell and watching postgresql.log I found out the following:

Method a)

list = Article.objects.filter(id__gt=130).order_by('-id')[0:2]
list[0] # Performs SQL query with LIMIT 1
list[1] # Performs another SQL query with LIMIT 1 OFFSET 2

Method b)

list = Article.objects.filter(id__gt=130).order_by('-id')[0:2]
list # Performs an SQL query with LIMIT 2
list[0] # Hits the cache
list[1] # Hits the cache

Calling list[0] afterwards will invoke the SQL query with LIMIT 1

Calling list[1] after this will invoke another query with LIMT 1 OFFSET 1

b) Calling just "list" afterwards first and then calling list[0] and list[1]

will honor the cache and therefore only one SQL query gets fired.

Change History (9)

comment:1 Changed 7 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Ouch, sorry, text wasn't formatted properly.

Method a)

list = Article.objects.filter(idgt=130).order_by('-id')[0:2]
list[0] # Performs SQL query with LIMIT 1
list[1] # Performs another SQL query with LIMIT 1 OFFSET 2

Method b)

list = Article.objects.filter(idgt=130).order_by('-id')[0:2]
list # Performs an SQL query with LIMIT 2
list[0] # Hits the cache
list[1] # Hits the cache

comment:2 Changed 7 years ago by lars@…

It seems that QuerySet.getitem assumes that there is a cache (result_cache) already. This cache gets initialized/filled by calling repr or len since they trigger the iter which calls QuerySet.iterator. Calling getitem without repr or len before will not initialize the cache and will lead to subsequent select queries.

I would expect that

list = Article.objects.filter(idgt=130).order_by('-id')[0:2]

calls performs a select with LIMIT 2 on the database and fills the cache with two enties, so that when I would call list[0] and list[1] would fetch their data from the cache and not from a SQL select.

comment:3 Changed 7 years ago by anonymous

  • Summary changed from queryset-rf does not always honor cache to Slicing on QuerySet bypasses result_cache

comment:4 Changed 7 years ago by gav

  • Keywords qsrf-cleanup added

comment:5 Changed 7 years ago by ramiro

  • Description modified (diff)

comment:6 Changed 7 years ago by jacob

  • milestone set to 1.0

comment:7 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Design decision needed

This isn't necessarily a bug. In fact, queryset slicing has always worked like this. If you add a slice to a sliced queryset, it restricts the results further, if it hasn't already populated the cache. And a single index access has always been treated the same as [k : k+1] (the behaviour was preserved in the queryset-rewrite so as to maintain backwards-compatibility).

I'm not going to change anything here until there's been some discussion and a clear resolution on django-dev about this, since the current behaviour actually makes sense in some situations and isn't clearly wrong.

comment:8 Changed 7 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

I'm going to close this as wontfix for now, since it's the way Django has always behaved. Personally, I think it's an abuse of single indexing to treat it as a slice and unintuitive, but that's the way things were designed. If somebody wants to bring it up on django-dev and there's a consensus to change, this can be reopened.

comment:9 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top