[Django] #31530: Check that CheckConstraint.check and UniqueConstraint.condition don't span joins.
Django
joins.
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: nobody
Charette |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Similar to #31410 but for `check` and `condition`.
---
Not everyone is familiar with the fact database level constraint cannot
span across tables and might be tempted to do
{{{#!python
class Person(models.Model):
age = models.PositiveSmallIntegerField()
parent = models.ForeignKey(self)
class Meta:
constraints = {
CheckConstraint(
name='age_lt_parent', check=Q(age__lt=parent__age)
),
}
}}}
Which we'll happily create migrations for but we'll then crash because we
prevent JOINs when resolving `check`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31530>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
joins.
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Old description:
> Similar to #31410 but for `check` and `condition`.
>
> ---
>
> Not everyone is familiar with the fact database level constraint cannot
> span across tables and might be tempted to do
>
> {{{#!python
> class Person(models.Model):
> age = models.PositiveSmallIntegerField()
> parent = models.ForeignKey(self)
>
> class Meta:
> constraints = {
> CheckConstraint(
> name='age_lt_parent', check=Q(age__lt=parent__age)
> ),
> }
> }}}
>
> Which we'll happily create migrations for but we'll then crash because we
> prevent JOINs when resolving `check`.
New description:
Similar to #31410 but for `check` and `condition`.
----
Not everyone is familiar with the fact database level constraint cannot
span across tables and might be tempted to do
{{{#!python
class Person(models.Model):
age = models.PositiveSmallIntegerField()
parent = models.ForeignKey(self)
class Meta:
constraints = {
CheckConstraint(
name='age_lt_parent', check=Q(age__lt=parent__age)
),
}
}}}
Which we'll happily create migrations for but we'll then crash because we
prevent JOINs when resolving `check`.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:1>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Hasan Ramezani
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/12953 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:2>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Is it even possible to handle all cases with Q objects?
As mentioned [https://code.djangoproject.com/ticket/31410#comment:11 in
this comment], custom expressions might be impossible to introspect.
Maybe we can rely on something like `get_source_expressions` to get all
included fields...
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:3>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hasan Ramezani):
Here is my
[https://github.com/django/django/pull/12953#discussion_r430551425 comment
on PR]:
Yes, this doesn't work for `Lower('parent__name')`. I can change the patch
to cover this case as well but I don't know exactly which kind of
expression should I cover? and with more cases, the code is going to be
complicated.
@felixxm Do you have any idea?
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:4>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:5>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* version: 3.0 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:6>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:7>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:8>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b7b7df5fbcf44e6598396905136cab5a19e9faff" b7b7df5]:
{{{
#!CommitTicketReference repository=""
revision="b7b7df5fbcf44e6598396905136cab5a19e9faff"
Fixed #31530 -- Added system checks for invalid model field names in
CheckConstraint.check and UniqueConstraint.condition.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:9>
Django
joins.
-------------------------------------+-------------------------------------
Type: | Ramezani
Cleanup/optimization | Status: closed
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"33abc55601107e9f12db3f0c16b3498b26c445f2" 33abc556]:
{{{
#!CommitTicketReference repository=""
revision="33abc55601107e9f12db3f0c16b3498b26c445f2"
Refs #31530 -- Added test for joined OneToOneField in
CheckConstraint.check
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31530#comment:10>