From 779cd28acb1f7eb06f629c0ea4ded99b5ebb670a Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Fri, 22 Sep 2023 06:01:11 +0200 Subject: [PATCH] Fixed #34840 -- Avoided casting string base fields on PostgreSQL. Thanks Alex Vandiver for the report. Regression in 09ffc5c1212d4ced58b708cbbf3dfbfb77b782ca. --- django/db/backends/postgresql/operations.py | 8 ----- django/db/models/lookups.py | 12 ++++--- docs/releases/4.2.6.txt | 10 ++++++ tests/backends/postgresql/tests.py | 14 ++++++++ tests/constraints/tests.py | 36 +++++++++++++++++++++ tests/lookup/tests.py | 10 ++++++ 6 files changed, 78 insertions(+), 12 deletions(-) diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py index 93eb982f4b..4a42126e81 100644 --- a/django/db/backends/postgresql/operations.py +++ b/django/db/backends/postgresql/operations.py @@ -154,14 +154,6 @@ class DatabaseOperations(BaseDatabaseOperations): def lookup_cast(self, lookup_type, internal_type=None): lookup = "%s" - - if lookup_type == "isnull" and internal_type in ( - "CharField", - "EmailField", - "TextField", - ): - return "%s::text" - # Cast text lookups to text to allow things like filter(x__contains=4) if lookup_type in ( "iexact", diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 896d3eee44..a562a3db38 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -624,11 +624,15 @@ class IsNull(BuiltinLookup): raise ValueError( "The QuerySet value for an isnull lookup must be True or False." ) - if isinstance(self.lhs, Value) and self.lhs.value is None: - if self.rhs: - raise FullResultSet + if isinstance(self.lhs, Value): + if self.lhs.value is None or ( + self.lhs.value == "" + and connection.features.interprets_empty_strings_as_nulls + ): + result_exception = FullResultSet if self.rhs else EmptyResultSet else: - raise EmptyResultSet + result_exception = EmptyResultSet if self.rhs else FullResultSet + raise result_exception sql, params = self.process_lhs(compiler, connection) if self.rhs: return "%s IS NULL" % sql, params diff --git a/docs/releases/4.2.6.txt b/docs/releases/4.2.6.txt index 23d5f2a04f..d89b5ff6b1 100644 --- a/docs/releases/4.2.6.txt +++ b/docs/releases/4.2.6.txt @@ -12,3 +12,13 @@ Bugfixes * Fixed a regression in Django 4.2.5 where overriding the deprecated ``DEFAULT_FILE_STORAGE`` and ``STATICFILES_STORAGE`` settings in tests caused the main ``STORAGES`` to mutate (:ticket:`34821`). + +* Fixed a regression in Django 4.2 that caused unnecessary casting of string + based fields (``CharField``, ``EmailField``, ``TextField``, ``CICharField``, + ``CIEmailField``, and ``CITextField``) used with the ``__isnull`` lookup on + PostgreSQL. As a consequence, the pre-Django 4.2 indexes didn't match and + were not used by the query planner (:ticket:`34840`). + + You may need to recreate indexes propagated to the database with Django + 4.2 - 4.2.5 as they contain unnecessary ``::text`` casting that is avoided as + of this release. diff --git a/tests/backends/postgresql/tests.py b/tests/backends/postgresql/tests.py index 31aac022c8..a045195991 100644 --- a/tests/backends/postgresql/tests.py +++ b/tests/backends/postgresql/tests.py @@ -369,6 +369,20 @@ class Tests(TestCase): with self.subTest(lookup=lookup): self.assertIn("::text", do.lookup_cast(lookup)) + def test_lookup_cast_isnull_noop(self): + from django.db.backends.postgresql.operations import DatabaseOperations + + do = DatabaseOperations(connection=None) + # Using __isnull lookup doesn't require casting. + tests = [ + "CharField", + "EmailField", + "TextField", + ] + for field_type in tests: + with self.subTest(field_type=field_type): + self.assertEqual(do.lookup_cast("isnull", field_type), "%s") + def test_correct_extraction_psycopg_version(self): from django.db.backends.postgresql.base import Database, psycopg_version diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index 55397449d9..e1b95baf60 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -995,6 +995,42 @@ class UniqueConstraintTests(TestCase): exclude={"name"}, ) + def test_validate_nullable_textfield_with_isnull_true(self): + is_null_constraint = models.UniqueConstraint( + "price", + "discounted_price", + condition=models.Q(unit__isnull=True), + name="uniq_prices_no_unit", + ) + is_not_null_constraint = models.UniqueConstraint( + "price", + "discounted_price", + condition=models.Q(unit__isnull=False), + name="uniq_prices_unit", + ) + + Product.objects.create(price=2, discounted_price=1) + Product.objects.create(price=4, discounted_price=3, unit="ng/mL") + + msg = "Constraint “uniq_prices_no_unit” is violated." + with self.assertRaisesMessage(ValidationError, msg): + is_null_constraint.validate( + Product, Product(price=2, discounted_price=1, unit=None) + ) + is_null_constraint.validate( + Product, Product(price=2, discounted_price=1, unit="ng/mL") + ) + is_null_constraint.validate(Product, Product(price=4, discounted_price=3)) + + msg = "Constraint “uniq_prices_unit” is violated." + with self.assertRaisesMessage(ValidationError, msg): + is_not_null_constraint.validate( + Product, + Product(price=4, discounted_price=3, unit="μg/mL"), + ) + is_not_null_constraint.validate(Product, Product(price=4, discounted_price=3)) + is_not_null_constraint.validate(Product, Product(price=2, discounted_price=1)) + def test_name(self): constraints = get_constraints(UniqueConstraintProduct._meta.db_table) expected_name = "name_color_uniq" diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index 398cc89cab..8af459ccd7 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -1337,6 +1337,16 @@ class LookupTests(TestCase): with self.assertRaisesMessage(ValueError, msg): qs.exists() + def test_isnull_textfield(self): + self.assertSequenceEqual( + Author.objects.filter(bio__isnull=True), + [self.au2], + ) + self.assertSequenceEqual( + Author.objects.filter(bio__isnull=False), + [self.au1], + ) + 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)