From 4bbc27c8686f10f9556cef02dbfa9f5157fbcf56 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 5 Jul 2025 21:19:05 -0400 Subject: Fixed #36492 -- Restored exact boolean lookup against literals on SQLite. Performance regression in 37e6c5b on SQLite. Just like MySQL, and presumably Oracle, which don't have a native boolean type and incidently store booleans in integer columns, indices on such columns cannot be used when explicit boolean literal equalities are omitted. Adapt the logic introduced by refs #32691 for MySQL to be used for all backends that don't support native boolean fields instead of special casing MySQL, SQLite, and Oracle in their own special way. Note that review of this work surfaced that SQLite's query planner also cannot make use of indices when dealing with expressions of form WHERE NOT (indexed_bool_field = false) but that's a long standing problem unrelated to the restorative work performed in this patch. Thanks Klaas van Schelven for the report. --- tests/lookup/tests.py | 66 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 18 deletions(-) (limited to 'tests') diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index b154541e78..9314fa05b0 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -2,7 +2,7 @@ import collections.abc from datetime import datetime from math import ceil from operator import attrgetter -from unittest import mock, skipUnless +from unittest import mock from django.core.exceptions import FieldError from django.db import connection, models @@ -19,6 +19,7 @@ from django.db.models import ( Value, When, ) +from django.db.models.expressions import RawSQL from django.db.models.functions import Abs, Cast, Length, Substr from django.db.models.lookups import ( Exact, @@ -29,7 +30,7 @@ from django.db.models.lookups import ( LessThan, LessThanOrEqual, ) -from django.test import TestCase, skipUnlessDBFeature +from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature from django.test.utils import ignore_warnings, isolate_apps, register_lookup from django.utils.deprecation import RemovedInDjango70Warning @@ -1591,10 +1592,10 @@ class LookupTests(TestCase): with self.assertRaisesMessage(ValueError, msg): list(Article.objects.filter(author=Author.objects.all()[1:])) - @skipUnless(connection.vendor == "mysql", "MySQL-specific workaround.") + @skipIfDBFeature("has_native_boolean_field") def test_exact_booleanfield(self): - # MySQL ignores indexes with boolean fields unless they're compared - # directly to a boolean value. + # Most databases without a native boolean type ignore indexes on them + # unless they're compared directly to a literal value. product = Product.objects.create(name="Paper", qty_target=5000) Stock.objects.create(product=product, short=False, qty_available=5100) stock_1 = Stock.objects.create(product=product, short=True, qty_available=180) @@ -1605,31 +1606,60 @@ class LookupTests(TestCase): str(qs.query), ) - @skipUnless(connection.vendor == "mysql", "MySQL-specific workaround.") + @skipIfDBFeature("has_native_boolean_field") def test_exact_booleanfield_annotation(self): - # MySQL ignores indexes with boolean fields unless they're compared - # directly to a boolean value. - qs = Author.objects.annotate( - case=Case( - When(alias="a1", then=True), - default=False, + # Most databases without a native boolean type ignore indexes on them + # unless they're compared directly to a literal value. + product = Product.objects.create(name="Paper", qty_target=5000) + Stock.objects.create(product=product, short=False, qty_available=5100) + stock_1 = Stock.objects.create(product=product, short=True, qty_available=180) + qs = Stock.objects.annotate( + short_annotation=F("short"), + ).filter(short_annotation=True) + self.assertSequenceEqual(qs, [stock_1]) + self.assertIn(" = True", str(qs.query)) + # ExpressionWrapper should be unwrapped. + qs = Stock.objects.annotate( + short_wrapper=ExpressionWrapper( + F("short"), output_field=BooleanField(), ) - ).filter(case=True) - self.assertSequenceEqual(qs, [self.au1]) + ).filter(short_wrapper=True) + self.assertSequenceEqual(qs, [stock_1]) self.assertIn(" = True", str(qs.query)) - + # Q which resolve to WhereNode should not be compared to a boolean + # value as it's compatible by definition. qs = Author.objects.annotate( - wrapped=ExpressionWrapper(Q(alias="a1"), output_field=BooleanField()), - ).filter(wrapped=True) + node=Q(alias="a1"), + ).filter(node=True) self.assertSequenceEqual(qs, [self.au1]) - self.assertIn(" = True", str(qs.query)) + self.assertNotIn(" = True", str(qs.query)) # EXISTS(...) shouldn't be compared to a boolean value. qs = Author.objects.annotate( exists=Exists(Author.objects.filter(alias="a1", pk=OuterRef("pk"))), ).filter(exists=True) self.assertSequenceEqual(qs, [self.au1]) self.assertNotIn(" = True", str(qs.query)) + # CASE shouldn't be compared to a boolean value. + qs = Author.objects.annotate( + case=Case( + When(alias="a1", then=True), + default=False, + output_field=BooleanField(), + ) + ).filter(case=True) + self.assertSequenceEqual(qs, [self.au1]) + self.assertEqual(str(qs.query).count(" = True"), 1) + # Conditional usage of RawSQL usage should not be compared to a boolean + # value. + queryset = Author.objects.all() + compiler = queryset.query.get_compiler(connection=connection) + sql, params = compiler.compile(Q(alias="a1").resolve_expression(queryset.query)) + qs = Author.objects.alias( + raw=RawSQL(sql, params, BooleanField()), + ).filter(raw=True) + self.assertSequenceEqual(qs, [self.au1]) + self.assertNotIn(" = True", str(qs.query)) def test_custom_field_none_rhs(self): """ -- cgit v1.3