[Django] #29195: Postgres Regression, annotating Exists through a OuterRef query.
Django
-------------------------------------+-------------------------------------
Reporter: Oli | Owner: nobody
Warner |
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
First proper report here, sorry if I mess it up. I've been using some
fairly advance subqueries since they were released but on upgrading a
mature-ish 1.11 site to 2.0 I notice that some of these broke. I've
distilled this down to a very simple pair of models and a test. This
passes on SQLite, fails on Postgres 10 and 9.6. Worked on both in Django
1.11.
All I'm trying to do here is find Booking instances that have any Payments
linked to them.
{{{#!python
from django.db import models
class Booking(models.Model):
pass
class Payment(models.Model):
booking = models.ForeignKey('Booking', models.CASCADE)
}}}
{{{#!python
from django.test import TestCase
from django.db.models import Exists, OuterRef
from .models import Booking, Payment
class AnnotateTestCase(TestCase):
def setUp(self):
self.booking_one = Booking.objects.create()
self.booking_two = Booking.objects.create()
Payment.objects.create(booking=self.booking_one)
def test_annotation(self):
# find bookings with payments
payment_query =
Payment.objects.order_by().filter(booking=OuterRef('pk')).values('booking')
booking_query =
Booking.objects.all().annotate(has_payments=Exists(payment_query)).filter(has_payments=True)
self.assertEqual(booking_query.count(), 1)
}}}
I've also tested the master branch. Same problem. Here's the trace.
{{{
Traceback (most recent call last):
File "/home/oli/Desktop/testsite/testsite/myapp/tests.py", line 19, in
test_annotation
self.assertEqual(booking_query.count(), 1)
File "/home/oli/Desktop/testsite/django/django/db/models/query.py", line
382, in count
return self.query.get_count(using=self.db)
File "/home/oli/Desktop/testsite/django/django/db/models/sql/query.py",
line 494, in get_count
number = obj.get_aggregation(using, ['__count'])['__count']
File "/home/oli/Desktop/testsite/django/django/db/models/sql/query.py",
line 479, in get_aggregation
result = compiler.execute_sql(SINGLE)
File
"/home/oli/Desktop/testsite/django/django/db/models/sql/compiler.py", line
1053, in execute_sql
cursor.execute(sql, params)
File "/home/oli/Desktop/testsite/django/django/db/backends/utils.py",
line 68, in execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File "/home/oli/Desktop/testsite/django/django/db/backends/utils.py",
line 77, in _execute_with_wrappers
return executor(sql, params, many, context)
File "/home/oli/Desktop/testsite/django/django/db/backends/utils.py",
line 85, in _execute
return self.cursor.execute(sql, params)
File "/home/oli/Desktop/testsite/django/django/db/utils.py", line 89, in
__exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/home/oli/Desktop/testsite/django/django/db/backends/utils.py",
line 85, in _execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: operator does not exist: boolean =
integer
LINE 1: ...0 WHERE U0."booking_id" = ("myapp_booking"."id")) = 1 GROUP ...
^
HINT: No operator matches the given name and argument type(s). You might
need to add explicit type casts.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29195>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
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 Oli Warner):
I ran a bisect all the way back to 301de774c2 (April 17, when I knew it
worked). I altered the test so that `git bisect run` would only consider a
bad result one that contained a message about "explicit type casts". The
first commit to break this was:
{{{
commit 08654a99bbdd09049d682ae57cc94241534b29f0
Author: Simon Charette <chare...@gmail.com>
Date: Tue Aug 8 13:31:59 2017 -0400
Fixed #28492 -- Defined default output_field of expressions at the
class level.
This wasn't possible when settings were accessed during Field
initialization
time as our test suite setup script was triggering imports of
expressions
before settings were configured.
}}}
I'm really quite a distance out of my comfort zone here —I don't usually
spend this much time in the guts of Django— so even looking at the bisect
visualisation, I'm not sure what change actually broke this. Still, I hope
this helps.
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Oli Warner):
This can be worked around by explicitly casting with `output_field `. The
following works:
{{{
Booking.objects.all().annotate(has_payments=Exists(payment_query,
output_field=BooleanField())).filter(has_payments=True)
}}}
... But it shouldn't be necessary.
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Hey Oli, thank you for your report!
I'm not sure what is at play here but if you bisected the issue to this
commit and it's possible to work around it using an explicit
`output_field` it's probably an issue with expression cloning.
Would it be possible to include the fully generated queries that crashes
on PostgreSQL, it should be doable using the ` --debug-sql` flag to the
test command.
I didn't reproduce but accepting based on the detailed error report.
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:3>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Simon Charette
* status: new => assigned
Comment:
I figured it out.
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:4>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/9765
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:5>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Oli Warner):
Thank you, Simon. I really appreciate you picking this up so fast and
fixing it.
I've tested your branch and it does fix my little test harness too.
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:6>
Django
-------------------------------------+-------------------------------------
| Charette
Component: Database layer | Version: 2.0
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"277ed072094ad87fc6b2c4669f21d43b1f39043c" 277ed072]:
{{{
#!CommitTicketReference repository=""
revision="277ed072094ad87fc6b2c4669f21d43b1f39043c"
Fixed #29195 -- Fixed Exists.output_field resolution on single-valued
queries.
The Subquery class which Exists inherits from defaulted to using single-
valued
querie's field if no output_field was explicitly specified on
initialization
which was bypassing the Exists.output_field defined at the class level.
Moving Subquery's dynamic output_field resolution to _resolve_output_field
should make sure the fallback logic is only performed if required.
Regression in 08654a99bbdd09049d682ae57cc94241534b29f0.
Thanks Oli Warner for the detailed report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:7>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"0fd21febe7b3969460e3e6f17bfedc63529f194b" 0fd21feb]:
{{{
#!CommitTicketReference repository=""
revision="0fd21febe7b3969460e3e6f17bfedc63529f194b"
[2.0.x] Fixed #29195 -- Fixed Exists.output_field resolution on single-
valued queries.
The Subquery class which Exists inherits from defaulted to using single-
valued
querie's field if no output_field was explicitly specified on
initialization
which was bypassing the Exists.output_field defined at the class level.
Moving Subquery's dynamic output_field resolution to _resolve_output_field
should make sure the fallback logic is only performed if required.
Regression in 08654a99bbdd09049d682ae57cc94241534b29f0.
Thanks Oli Warner for the detailed report.
Backport of 277ed072094ad87fc6b2c4669f21d43b1f39043c from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29195#comment:8>