Opened 11 years ago

Closed 6 years ago

#21703 closed Bug (fixed)

`exclude` query with `F` object across relationship fails

Reported by: anonymous Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given the following models..

Code highlighting: 

  class Owner(models.Model):
    pass
 
 
class Thing(models.Model):
    owner = models.ForeignKey(Owner, related_name='things')
 
    date = models.DateField()
 
 
class Error(models.Model):
    owner = models.ForeignKey(Owner, related_name='errors')
 
    date = models.DateField()

The following exclude query fails.

Code highlighting: 

  >>> Error.objects.exclude(owner__things__date=F('date'))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/query.py", line 115, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/query.py", line 140, in __iter__
    self._fetch_all()
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/query.py", line 962, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/query.py", line 264, in iterator
    for row in compiler.results_iter():
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 681, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 762, in execute_sql
    cursor.execute(sql, params)
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/backends/utils.py", line 77, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/utils.py", line 93, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
  File "/Users/foobar/python-environments/358426a27f281ff5/lib/python2.7/site-packages/django/db/backends/sqlite3/base.py", line 489, in execute
    return Database.Cursor.execute(self, query, params)
OperationalError: no such column: U1.id

The same query using a filter works as expected. This bug is present in 1.5.5, 1.6.1 and 1.7.dev20131229181725

Change History (11)

comment:1 by Piper Merriam, 11 years ago

This bug is also confirmed present on the mysql backend.

comment:2 by Piper Merriam, 11 years ago

Component: Uncategorized → Database layer (models, ORM)
Type: Uncategorized → Bug

comment:3 by Jonas Kölker, 11 years ago

This seems to be related to, if not the same as, Bug #18726 (see especially ticket:18726#comment:8).

comment:4 by Baptiste Mispelon, 11 years ago

Triage Stage: Unreviewed → Accepted

I can reproduce this too.

I'm not sure if it's the same bug as #18726 (the error messages are different) but it might be related.

#21787 is another ticket where filter() works but exclude() breaks (this one is actually a regression which I traced back to b4492a8ca4a7ae4daa3a6b03c3d7a845fad74931).

comment:5 by Anssi Kääriäinen, 11 years ago

This one is complex to solve. In the query F('date') must refer the outer query's date field, not anything in the inner query. The way split_exclude() is implemented is to add .filter(ownre__things__date=F('date')) in the inner query, then trimming prefixes. We could add the F() into the outer query and pass that value to the inner query. Unfortunately that doesn't work if the F() object references something in the outer instead of inner query (for example F('owner__things__somecol')). In that case we must add the F() to the outer query. This gets even more complicated if you use F('date') + F('owner__things__somecol'). Neither inner nor outer query addition works for the whole expression, the first F() needs to refer to outer, the second to inner query.

So, we need bigger changes into the ORM. I think we need to teach setup_joins() to auto-refer to the right query based on prefix, and rework how split_exclude() works. I don't have good ideas of how to actually achieve all this. It seems implementing this is going to need a lot of work.

There are other complex cases to be solved for .split_exclude() - for example .exclude(owner__things__date__gte=date1, owner__things__date__lte=date2) doesn't work currently, and I suspect there are similar problems if you use complex boolean trees where some parts refer to outer query, some parts to inner. The most complex case is to have references to two different m2m joins, something like: .exclude(owner__things__date__gte=date1, creator__things__date__lte=date2). I am not even sure how to write that in SQL using subqueries...

It seems #18726 is at least close to this, if not duplicate. I haven't done enough research to be certain.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:6 by Tim Graham, 7 years ago

I closed #28551 as a duplicate (same problem query across a many-to-many relation).

comment:7 by Simon Charette, 6 years ago

The way split_exclude() is implemented is to add filter(owner__things__date=F('date')) in the inner query, ...

I wonder if this could be fixed by making split_exclude() add filter(owner__things__date=OuterRef('date')) in the inner query instead.

comment:8 by Simon Charette, 6 years ago

I confirmed this is fixed by relying on OuterRef

diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 7d991b6b84..458ecaa02a 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -19,7 +19,9 @@ from django.core.exceptions import (
 from django.db import DEFAULT_DB_ALIAS, NotSupportedError, connections
 from django.db.models.aggregates import Count
 from django.db.models.constants import LOOKUP_SEP
-from django.db.models.expressions import Col, F, Ref, SimpleCol
+from django.db.models.expressions import (
+    Col, F, OuterRef, Ref, SimpleCol, Subquery,
+)
 from django.db.models.fields import Field
 from django.db.models.fields.related_lookups import MultiColSource
 from django.db.models.lookups import Lookup
@@ -1630,6 +1632,8 @@ class Query:
         saner null handling, and is easier for the backend's optimizer to
         handle.
         """
+        if isinstance(filter_expr[1], F):
+            filter_expr = (filter_expr[0], OuterRef(filter_expr[1].name))
         # Generate the inner query.
         query = Query(self.model)
         query.add_filter(filter_expr)
@@ -1662,8 +1666,10 @@ class Query:
             query.where.add(lookup, AND)
             query.external_aliases.add(alias)

+        from django.db import models
+        queryset = models.QuerySet(self.model, query)
         condition, needed_inner = self.build_filter(
-            ('%s__in' % trimmed_prefix, query),
+            ('%s__in' % trimmed_prefix, Subquery(queryset)),
             current_negated=True, branch_negated=True, can_reuse=can_reuse)
         if contains_louter:
             or_null_condition, _ = self.build_filter(

But this makes Query depend on QuerySet which we want to avoid. I have always thought Subquery wrapping of QuerySet instances was unnecessary and that OuterRef ought to be resolved by Query anyway so I'll give it a try.

Version 0, edited 6 years ago by Simon Charette (next)

comment:9 by Simon Charette, 6 years ago

Needs tests: set

Addressed by ​https://github.com/django/django/pull/11062

comment:10 by Tim Graham <timograham@…>, 6 years ago

In 35431298:

Refs #27149 -- Moved subquery expression resolving to Query.

This makes Subquery a thin wrapper over Query and makes sure it respects
the Expression source expression API by accepting the same number of
expressions as it returns. Refs #30188.

It also makes OuterRef usable in Query without Subquery wrapping. This
should allow Query's internals to more easily perform subquery push downs
during split_exclude(). Refs #21703.

comment:11 by Tim Graham <timograham@…>, 6 years ago

Resolution: → fixed
Status: new → closed

In f19a4945:

Fixed #21703 -- Fixed a crash when excluding a related field with a F().

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