From 7991111af12056ec9a856f35935d273526338c1f Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 10 Jul 2019 10:33:36 +0200 Subject: [PATCH] Fixed #30621 -- Fixed crash of __contains lookup for Date/DateTimeRangeField when the right hand side is the same type. Thanks Tilman Koschnick for the report and initial patch. Thanks Carlton Gibson the review. Regression in 6b048b364ca1e0e56a0d3815bf2be33ac9998355. --- django/contrib/postgres/fields/ranges.py | 7 ++- docs/releases/2.2.4.txt | 6 +++ .../migrations/0002_create_test_models.py | 2 + tests/postgres_tests/models.py | 2 + tests/postgres_tests/test_constraints.py | 52 ++++++++++++++++++- tests/postgres_tests/test_ranges.py | 10 +++- 6 files changed, 76 insertions(+), 3 deletions(-) diff --git a/django/contrib/postgres/fields/ranges.py b/django/contrib/postgres/fields/ranges.py index 74ba4eb230..0e8a347d5f 100644 --- a/django/contrib/postgres/fields/ranges.py +++ b/django/contrib/postgres/fields/ranges.py @@ -170,7 +170,12 @@ class DateTimeRangeContains(models.Lookup): params = lhs_params + rhs_params # Cast the rhs if needed. cast_sql = '' - if isinstance(self.rhs, models.Expression) and self.rhs._output_field_or_none: + if ( + isinstance(self.rhs, models.Expression) and + self.rhs._output_field_or_none and + # Skip cast if rhs has a matching range type. + not isinstance(self.rhs._output_field_or_none, self.lhs.output_field.__class__) + ): cast_internal_type = self.lhs.output_field.base_field.get_internal_type() cast_sql = '::{}'.format(connection.data_types.get(cast_internal_type)) return '%s @> %s%s' % (lhs, rhs, cast_sql), params diff --git a/docs/releases/2.2.4.txt b/docs/releases/2.2.4.txt index a1a849680d..0ad92f4ab1 100644 --- a/docs/releases/2.2.4.txt +++ b/docs/releases/2.2.4.txt @@ -12,3 +12,9 @@ Bugfixes * Fixed a regression in Django 2.2 when ordering a ``QuerySet.union()``, ``intersection()``, or ``difference()`` by a field type present more than once results in the wrong ordering being used (:ticket:`30628`). + +* Fixed a migration crash on PostgreSQL when adding a check constraint + with a ``contains`` lookup on + :class:`~django.contrib.postgres.fields.DateRangeField` or + :class:`~django.contrib.postgres.fields.DateTimeRangeField`, if the right + hand side of an expression is the same type (:ticket:`30621`). diff --git a/tests/postgres_tests/migrations/0002_create_test_models.py b/tests/postgres_tests/migrations/0002_create_test_models.py index dc941de139..b9f9cee6bf 100644 --- a/tests/postgres_tests/migrations/0002_create_test_models.py +++ b/tests/postgres_tests/migrations/0002_create_test_models.py @@ -211,7 +211,9 @@ class Migration(migrations.Migration): ('bigints', BigIntegerRangeField(null=True, blank=True)), ('decimals', DecimalRangeField(null=True, blank=True)), ('timestamps', DateTimeRangeField(null=True, blank=True)), + ('timestamps_inner', DateTimeRangeField(null=True, blank=True)), ('dates', DateRangeField(null=True, blank=True)), + ('dates_inner', DateRangeField(null=True, blank=True)), ], options={ 'required_db_vendor': 'postgresql' diff --git a/tests/postgres_tests/models.py b/tests/postgres_tests/models.py index 2bb6e6fcdf..3d170a9a1a 100644 --- a/tests/postgres_tests/models.py +++ b/tests/postgres_tests/models.py @@ -135,7 +135,9 @@ class RangesModel(PostgreSQLModel): bigints = BigIntegerRangeField(blank=True, null=True) decimals = DecimalRangeField(blank=True, null=True) timestamps = DateTimeRangeField(blank=True, null=True) + timestamps_inner = DateTimeRangeField(blank=True, null=True) dates = DateRangeField(blank=True, null=True) + dates_inner = DateRangeField(blank=True, null=True) class RangeLookupsModel(PostgreSQLModel): diff --git a/tests/postgres_tests/test_constraints.py b/tests/postgres_tests/test_constraints.py index 0e09a1c546..2fc6ee5322 100644 --- a/tests/postgres_tests/test_constraints.py +++ b/tests/postgres_tests/test_constraints.py @@ -1,5 +1,7 @@ +import datetime + from django.db import connection, transaction -from django.db.models import Q +from django.db.models import F, Q from django.db.models.constraints import CheckConstraint from django.db.utils import IntegrityError @@ -33,3 +35,51 @@ class SchemaTests(PostgreSQLTestCase): with self.assertRaises(IntegrityError), transaction.atomic(): RangesModel.objects.create(ints=(20, 50)) RangesModel.objects.create(ints=(10, 30)) + + def test_check_constraint_daterange_contains(self): + constraint_name = 'dates_contains' + self.assertNotIn(constraint_name, self.get_constraints(RangesModel._meta.db_table)) + constraint = CheckConstraint( + check=Q(dates__contains=F('dates_inner')), + name=constraint_name, + ) + with connection.schema_editor() as editor: + editor.add_constraint(RangesModel, constraint) + with connection.cursor() as cursor: + constraints = connection.introspection.get_constraints(cursor, RangesModel._meta.db_table) + self.assertIn(constraint_name, constraints) + date_1 = datetime.date(2016, 1, 1) + date_2 = datetime.date(2016, 1, 4) + with self.assertRaises(IntegrityError), transaction.atomic(): + RangesModel.objects.create( + dates=(date_1, date_2), + dates_inner=(date_1, date_2.replace(day=5)), + ) + RangesModel.objects.create( + dates=(date_1, date_2), + dates_inner=(date_1, date_2), + ) + + def test_check_constraint_datetimerange_contains(self): + constraint_name = 'timestamps_contains' + self.assertNotIn(constraint_name, self.get_constraints(RangesModel._meta.db_table)) + constraint = CheckConstraint( + check=Q(timestamps__contains=F('timestamps_inner')), + name=constraint_name, + ) + with connection.schema_editor() as editor: + editor.add_constraint(RangesModel, constraint) + with connection.cursor() as cursor: + constraints = connection.introspection.get_constraints(cursor, RangesModel._meta.db_table) + self.assertIn(constraint_name, constraints) + datetime_1 = datetime.datetime(2016, 1, 1) + datetime_2 = datetime.datetime(2016, 1, 2, 12) + with self.assertRaises(IntegrityError), transaction.atomic(): + RangesModel.objects.create( + timestamps=(datetime_1, datetime_2), + timestamps_inner=(datetime_1, datetime_2.replace(hour=13)), + ) + RangesModel.objects.create( + timestamps=(datetime_1, datetime_2), + timestamps_inner=(datetime_1, datetime_2), + ) diff --git a/tests/postgres_tests/test_ranges.py b/tests/postgres_tests/test_ranges.py index ae834b6ff0..89f32ee77c 100644 --- a/tests/postgres_tests/test_ranges.py +++ b/tests/postgres_tests/test_ranges.py @@ -115,11 +115,15 @@ class TestRangeContainsLookup(PostgreSQLTestCase): ] cls.obj = RangesModel.objects.create( dates=(cls.dates[0], cls.dates[3]), + dates_inner=(cls.dates[1], cls.dates[2]), timestamps=(cls.timestamps[0], cls.timestamps[3]), + timestamps_inner=(cls.timestamps[1], cls.timestamps[2]), ) cls.aware_obj = RangesModel.objects.create( dates=(cls.dates[0], cls.dates[3]), + dates_inner=(cls.dates[1], cls.dates[2]), timestamps=(cls.aware_timestamps[0], cls.aware_timestamps[3]), + timestamps_inner=(cls.timestamps[1], cls.timestamps[2]), ) # Objects that don't match any queries. for i in range(3, 4): @@ -140,6 +144,7 @@ class TestRangeContainsLookup(PostgreSQLTestCase): (self.aware_timestamps[1], self.aware_timestamps[2]), Value(self.dates[0], output_field=DateTimeField()), Func(F('dates'), function='lower', output_field=DateTimeField()), + F('timestamps_inner'), ) for filter_arg in filter_args: with self.subTest(filter_arg=filter_arg): @@ -154,6 +159,7 @@ class TestRangeContainsLookup(PostgreSQLTestCase): (self.dates[1], self.dates[2]), Value(self.dates[0], output_field=DateField()), Func(F('timestamps'), function='lower', output_field=DateField()), + F('dates_inner'), ) for filter_arg in filter_args: with self.subTest(filter_arg=filter_arg): @@ -361,7 +367,9 @@ class TestSerialization(PostgreSQLSimpleTestCase): '\\"bounds\\": \\"[)\\"}", "decimals": "{\\"empty\\": true}", ' '"bigints": null, "timestamps": "{\\"upper\\": \\"2014-02-02T12:12:12+00:00\\", ' '\\"lower\\": \\"2014-01-01T00:00:00+00:00\\", \\"bounds\\": \\"[)\\"}", ' - '"dates": "{\\"upper\\": \\"2014-02-02\\", \\"lower\\": \\"2014-01-01\\", \\"bounds\\": \\"[)\\"}" }, ' + '"timestamps_inner": null, ' + '"dates": "{\\"upper\\": \\"2014-02-02\\", \\"lower\\": \\"2014-01-01\\", \\"bounds\\": \\"[)\\"}", ' + '"dates_inner": null }, ' '"model": "postgres_tests.rangesmodel", "pk": null}]' )