Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32635 closed Bug (fixed)

System checks for invalid model field names in CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020 relation.

Reported by: sim1234 Owned by: Hasan Ramezani
Component: Database layer (models, ORM) Version: 3.2
Severity: Release blocker Keywords: UniqueConstraint OneToOneField
Cc: Hasan Ramezani Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Creating a UniqueConstraint with condition referencing OneToOneField generates invalid SQL

Example:

# models.py
from django.db import models

class Model1(models.Model):
    name = models.CharField(max_length=255)

    class Meta:
        constraints = (
            models.UniqueConstraint(
                name="unique_onetoone",
                fields=("name", ),
                condition=models.Q(model__isnull=True),
            ),
        )

class Model2(models.Model):
    name = models.CharField(max_length=255)
    model = models.OneToOneField(
        Model1, related_name="model", on_delete=models.CASCADE
    )


# generated migrations/0001_initial.py
import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):
    dependencies = []

    operations = [
        // ... CreateModel, etc ...
        migrations.AddConstraint(
            model_name="model1",
            constraint=models.UniqueConstraint(
                name="unique_onetoone",
                fields=("name", ),
                condition=models.Q(model__isnull=True),
            ),
        ),
    ]

Running manage.py sqlmigrate test 0001 generates this SQL for creating the unique index and the WHERE clause is completly wrong.

-- ... CREATE TABLE, etc ...
CREATE UNIQUE INDEX "unique_onetoone" ON "test_model1" ("name", ) WHERE "id" IS NULL;

Expected behaviour is receiving FieldError("Joined field references are not permitted in this query"). This behaviour is observed when using condition=models.Q(modelidisnull=True).

Change History (7)

comment:1 by Simon Charette, 3 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I could swear this was discussed already but both conditions and expressions for Index and Constrant should be checked. This should be achievable through the use allow_joins=False at resolving time.

comment:2 by Mariusz Felisiak, 3 years ago

Cc: Hasan Ramezani added
Severity: NormalRelease blocker
Summary: Creating a UniqueConstraint with condition referencing OneToOneField generates invalid SQLSystem checks for invalid model field names in CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020 relation.
Version: 2.23.2

System checks for invalid model field names in CheckConstraint.check and UniqueConstraint.condition, added in b7b7df5fbcf44e6598396905136cab5a19e9faff, crash for me with a reverse 020 relation, e.g.

diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py
index c79684487d..f39e424251 100644
--- a/tests/invalid_models_tests/test_models.py
+++ b/tests/invalid_models_tests/test_models.py
@@ -1694,6 +1694,25 @@ class ConstraintsTests(TestCase):
             ),
         ])
 
+    @skipUnlessDBFeature('supports_table_check_constraints')
+    def test_check_constraint_pointing_to_reverse_o2o(self):
+        class Model(models.Model):
+            parent = models.OneToOneField('self', models.CASCADE, related_name='model')
+
+            class Meta:
+                constraints = [
+                    models.CheckConstraint(name='name', check=models.Q(model__isnull=True)),
+                ]
+
+        self.assertEqual(Model.check(databases=self.databases), [
+            Error(
+                "'constraints' refers to the nonexistent field 'model'.",
+                obj=Model,
+                id='models.E012',
+            ),
+        ])
+


  File "django/django/db/models/base.py", line 1296, in check
    *cls._check_constraints(databases),
  File "django/django/db/models/base.py", line 2108, in _check_constraints
    field.get_transform(first_lookup) is None and
AttributeError: 'OneToOneRel' object has no attribute 'get_transform'

Marking as a release blocker since it's a bug in a new feature.

comment:3 by Hasan Ramezani, 3 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

comment:4 by Hasan Ramezani, 3 years ago

comment:5 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In a77c9a4:

Fixed #32635 -- Fixed system check crash for reverse o2o relations in CheckConstraint.check and UniqueConstraint.condition.

Regression in b7b7df5fbcf44e6598396905136cab5a19e9faff.

Thanks Szymon Zmilczak for the report.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 700356f9:

[3.2.x] Fixed #32635 -- Fixed system check crash for reverse o2o relations in CheckConstraint.check and UniqueConstraint.condition.

Regression in b7b7df5fbcf44e6598396905136cab5a19e9faff.

Thanks Szymon Zmilczak for the report.

Backport of a77c9a4229cfef790ec18001b2cd18bd9c4aedbc from main

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