Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10670 closed (fixed)

Using a QuerySet in a filter expression can cause later evaluation of the QuerySet to raise ProgrammingError

Reported by: benanhalt Owned by: Alex
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Adding the following test case to tests/modeltests/aggregation/models.py:

>>> publishers = Publisher.objects.filter(id__in=(1,2))
>>> publishers
[<Publisher: Apress>, <Publisher: Sams>]

>>> publishers = publishers.annotate(n_books=models.Count('book'))
>>> publishers[0].n_books
2

>>> publishers # Here it is OK
[<Publisher: Apress>, <Publisher: Sams>]

>>> books = Book.objects.filter(publisher__in=publishers)
>>> books
[<Book: Sams Teach Yourself Django in 24 Hours>, <Book: Practical Django Projects>, <Book: The Definitive Guide to Django: Web Development Done Right>]

>>> publishers # Here it fails.
[<Publisher: Apress>, <Publisher: Sams>]

results in:

Failed example:
    publishers # Here it fails.
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python2.5/site-packages/django/test/_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest modeltests.aggregation.models.__test__.BUG[8]>", line 1, in <module>
        publishers # Here it fails.
      File "/usr/lib/python2.5/site-packages/django/db/models/query.py", line 55, in __repr__
        data = list(self[:REPR_OUTPUT_SIZE + 1])
      File "/usr/lib/python2.5/site-packages/django/db/models/query.py", line 70, in __len__
        self._result_cache.extend(list(self._iter))
      File "/usr/lib/python2.5/site-packages/django/db/models/query.py", line 193, in iterator
        for row in self.query.results_iter():
      File "/usr/lib/python2.5/site-packages/django/db/models/sql/query.py", line 262, in results_iter
        for rows in self.execute_sql(MULTI):
      File "/usr/lib/python2.5/site-packages/django/db/models/sql/query.py", line 2285, in execute_sql
        cursor.execute(sql, params)
    ProgrammingError: missing FROM-clause entry for table "U1"
    LINE 1: ...ame", "aggregation_publisher"."num_awards", COUNT("U1"."id")...

I'm using postgres.

Attachments (2)

aggregation-copy.diff (1.8 KB) - added by Alex 5 years ago.
aggregation-copy.2.diff (1.7 KB) - added by Alex 5 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by Alex

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This seems to be exposing 2 problems: a) Why isn't the query cache being used?, b) Why are we generating invalid SQL?

comment:2 Changed 5 years ago by mtredinnick

This is only one problem (the annotation stuff is doing something funky).

The queryset cache not being used is intentional: the sort of query construction demonstrated here is used to create nested queries. It intentionally avoids passing 10,000 id values or something to the "in" request by constructing nested SQL. When a queryset is used as an rvalue the queryset itself is thrown away pretty much immediately and only the inner Query object is interesting.

comment:3 Changed 5 years ago by Alex

Yeah, you're correct, I didn't think about that one clearly enough(obviously when the queryset clones itself in _as_sql it tosses out the result cache).

comment:4 Changed 5 years ago by Alex

The issue is probably that query.clone() doesn't copy aggregates. I'll write a patch.

Changed 5 years ago by Alex

comment:5 Changed 5 years ago by Alex

  • Has patch set

comment:6 Changed 5 years ago by Alex

Actually I lied about the query cache issue, I wasn't talking about using the query cache for the in filter, I meant when we reprint publishers, why isn't it keeping the query cache?

Changed 5 years ago by Alex

comment:7 Changed 5 years ago by Alex

  • Owner changed from nobody to Alex
  • Status changed from new to assigned

comment:8 Changed 5 years ago by Alex

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 5 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10357]) Fixed #10670: fixed reusing QuerySets previously used in a filter expression. Thanks, Alex Gaynor.

comment:10 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.