[Django] #32635: Creating a UniqueConstraint with condition referencing OneToOneField generates invalid SQL

10 views
Skip to first unread message

Django

unread,
Apr 12, 2021, 6:02:04 AM4/12/21
to django-...@googlegroups.com
#32635: Creating a UniqueConstraint with condition referencing OneToOneField
generates invalid SQL
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: nobody
Type: | Status: new
Uncategorized |
Component: | Version: 2.2
Uncategorized | Keywords: UniqueConstraint
Severity: Normal | OneToOneField
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
== 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(model__id__isnull=True)''.

--
Ticket URL: <https://code.djangoproject.com/ticket/32635>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 12, 2021, 9:54:59 AM4/12/21
to django-...@googlegroups.com
#32635: Creating a UniqueConstraint with condition referencing OneToOneField
generates invalid SQL
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: UniqueConstraint | Triage Stage: Accepted
OneToOneField |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* type: Uncategorized => Bug
* component: Uncategorized => Database layer (models, ORM)
* stage: Unreviewed => Accepted


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:1>

Django

unread,
Apr 12, 2021, 3:06:40 PM4/12/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.

-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: UniqueConstraint | Triage Stage: Accepted
OneToOneField |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* cc: Hasan Ramezani (added)
* version: 2.2 => 3.2
* severity: Normal => Release blocker


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:2>

Django

unread,
Apr 13, 2021, 3:11:43 AM4/13/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: Hasan
| Ramezani
Type: Bug | Status: assigned

Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: UniqueConstraint | Triage Stage: Accepted
OneToOneField |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

* owner: nobody => Hasan Ramezani
* status: new => assigned
* has_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:3>

Django

unread,
Apr 13, 2021, 3:37:18 AM4/13/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: Hasan
| Ramezani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: UniqueConstraint | Triage Stage: Accepted
OneToOneField |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Hasan Ramezani):

[https://github.com/django/django/pull/14255 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:4>

Django

unread,
Apr 14, 2021, 3:58:07 AM4/14/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: Hasan
| Ramezani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: UniqueConstraint | Triage Stage: Ready for
OneToOneField | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:5>

Django

unread,
Apr 14, 2021, 4:32:09 AM4/14/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: Hasan
| Ramezani
Type: Bug | Status: closed

Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed

Keywords: UniqueConstraint | Triage Stage: Ready for
OneToOneField | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"a77c9a4229cfef790ec18001b2cd18bd9c4aedbc" a77c9a4]:
{{{
#!CommitTicketReference repository=""
revision="a77c9a4229cfef790ec18001b2cd18bd9c4aedbc"
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.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:6>

Django

unread,
Apr 14, 2021, 4:32:47 AM4/14/21
to django-...@googlegroups.com
#32635: System checks for invalid model field names in
CheckConstraint.check/UniqueConstraint.condition crash with a reverse 020
relation.
-------------------------------------+-------------------------------------
Reporter: sim1234 | Owner: Hasan
| Ramezani
Type: Bug | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: UniqueConstraint | Triage Stage: Ready for
OneToOneField | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"700356f93b9185cc05d9ed349108591a70e597b6" 700356f9]:
{{{
#!CommitTicketReference repository=""
revision="700356f93b9185cc05d9ed349108591a70e597b6"
[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
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32635#comment:7>

Reply all
Reply to author
Forward
0 new messages