From 170b006ce82b0ecf26dc088f832538b747ca0115 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Thu, 22 Apr 2021 13:44:25 -0400 Subject: [PATCH] Fixed #32673 -- Fixed lookups crash when comparing against lookups on PostgreSQL. Regression in 3a505c70e7b228bf1212c067a8f38271ca86ce09. Nonlitteral right-hand-sides of lookups need to be wrapped in parentheses to avoid operator precedence ambiguities. Thanks Charles Lirsac for the detailed report. --- django/db/backends/oracle/features.py | 4 ++++ django/db/models/lookups.py | 8 +++++++- tests/lookup/models.py | 1 + tests/lookup/tests.py | 24 ++++++++++++++++++++++-- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index 07f16eee71..712c56de01 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -87,6 +87,10 @@ class DatabaseFeatures(BaseDatabaseFeatures): 'Raises ORA-00600: internal error code.': { 'model_fields.test_jsonfield.TestQuerying.test_usage_in_subquery', }, + "Oracle doesn't allow filters to be compared to another expression in " + "the WHERE clause.": { + 'lookup.tests.LookupTests.test_lookup_rhs', + }, } django_test_expected_failures = { # A bug in Django/cx_Oracle with respect to string handling (#23843). diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 6c923fe1a6..c55208598c 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -94,7 +94,13 @@ class Lookup: value = self.apply_bilateral_transforms(value) value = value.resolve_expression(compiler.query) if hasattr(value, 'as_sql'): - return compiler.compile(value) + sql, params = compiler.compile(value) + # Ensure expression is wrapped in parentheses to respect operator + # precedence but avoid double wrapping as it can be misinterpreted + # on some backends (e.g. subqueries on SQLite). + if sql and sql[0] != '(': + sql = '(%s)' % sql + return sql, params else: return self.get_db_prep_lookup(value, connection) diff --git a/tests/lookup/models.py b/tests/lookup/models.py index 6a0afaacde..2343c850c1 100644 --- a/tests/lookup/models.py +++ b/tests/lookup/models.py @@ -94,6 +94,7 @@ class Product(models.Model): class Stock(models.Model): product = models.ForeignKey(Product, models.CASCADE) + short = models.BooleanField(default=False) qty_available = models.DecimalField(max_digits=6, decimal_places=2) diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index 28ddc13aa0..e38eae087c 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -5,13 +5,16 @@ from operator import attrgetter from django.core.exceptions import FieldError from django.db import connection, models -from django.db.models import Exists, Max, OuterRef +from django.db.models import ( + BooleanField, Exists, ExpressionWrapper, F, Max, OuterRef, Q, +) from django.db.models.functions import Substr from django.test import TestCase, skipUnlessDBFeature from django.test.utils import isolate_apps from .models import ( - Article, Author, Freebie, Game, IsNullWithNoneAsRHS, Player, Season, Tag, + Article, Author, Freebie, Game, IsNullWithNoneAsRHS, Player, Product, + Season, Stock, Tag, ) @@ -1000,3 +1003,20 @@ class LookupTests(TestCase): with self.subTest(qs=qs): with self.assertRaisesMessage(ValueError, msg): qs.exists() + + def test_lookup_rhs(self): + product = Product.objects.create(name='GME', qty_target=5000) + stock_1 = Stock.objects.create(product=product, short=True, qty_available=180) + stock_2 = Stock.objects.create(product=product, short=False, qty_available=5100) + Stock.objects.create(product=product, short=False, qty_available=4000) + self.assertCountEqual( + Stock.objects.filter(short=Q(qty_available__lt=F('product__qty_target'))), + [stock_1, stock_2], + ) + self.assertCountEqual( + Stock.objects.filter(short=ExpressionWrapper( + Q(qty_available__lt=F('product__qty_target')), + output_field=BooleanField(), + )), + [stock_1, stock_2], + )