Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#10670 closed (fixed)

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

Reported by: Ben Anhalt Owned by: Alex Gaynor
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 Gaynor 8 years ago.
aggregation-copy.2.diff (1.7 KB) - added by Alex Gaynor 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Alex Gaynor

milestone: 1.1
Triage Stage: UnreviewedAccepted

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 8 years ago by Malcolm Tredinnick

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 8 years ago by Alex Gaynor

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 8 years ago by Alex Gaynor

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

Changed 8 years ago by Alex Gaynor

Attachment: aggregation-copy.diff added

comment:5 Changed 8 years ago by Alex Gaynor

Has patch: set

comment:6 Changed 8 years ago by Alex Gaynor

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 8 years ago by Alex Gaynor

Attachment: aggregation-copy.2.diff added

comment:7 Changed 8 years ago by Alex Gaynor

Owner: changed from nobody to Alex Gaynor
Status: newassigned

comment:8 Changed 8 years ago by Alex Gaynor

Triage Stage: AcceptedReady for checkin

comment:9 Changed 8 years ago by Jacob

Resolution: fixed
Status: assignedclosed

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

comment:10 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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