Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29195 closed Bug (fixed)

Postgres Regression, annotating Exists through a OuterRef query.

Reported by: Oli Warner Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 2.0
Severity: Release blocker Keywords:
Cc: Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

from django.db import models

class Booking(models.Model):
    pass

class Payment(models.Model):
    booking = models.ForeignKey('Booking', models.CASCADE)
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.

Change History (8)

comment:1 by Oli Warner, 6 years ago

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 <charette.s@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.

comment:2 by Oli Warner, 6 years ago

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.

comment:3 by Simon Charette, 6 years ago

Cc: Simon Charette added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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.

comment:4 by Simon Charette, 6 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

I figured it out.

comment:6 by Oli Warner, 6 years ago

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.

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

Resolution: fixed
Status: assignedclosed

In 277ed072:

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.

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

In 0fd21feb:

[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

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