[Django] #30910: Add system check warning on duplicate check constraints.

12 views
Skip to first unread message

Django

unread,
Oct 24, 2019, 7:44:16 PM10/24/19
to django-...@googlegroups.com
#30910: Add system check warning on duplicate check constraints.
------------------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Recently, due to an oversight when copying & pasting code, I created a
duplicate `CheckConstraint`. I had intended to modify the `check` argument
after pasting, but I made a mistake. In this specific instance, the
mistake was caught and fixed during a routine code review, but it made me
think this is something that could be warned against by Django so it is
caught early.

While a duplicate check constraint is legal SQL, it normally points to an
error on the part of the developer. We can issue a `Warning` in such a
case.

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

Django

unread,
Oct 24, 2019, 7:45:37 PM10/24/19
to django-...@googlegroups.com
#30910: Add system check warning on duplicate check constraints.
-------------------------------------+-------------------------------------

Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

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

* has_patch: 0 => 1


Comment:

https://github.com/django/django/pull/11973

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

Django

unread,
Oct 25, 2019, 1:36:33 AM10/25/19
to django-...@googlegroups.com
#30910: Add system check warning on duplicate check constraints.
--------------------------------------+------------------------------------

Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

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

* stage: Unreviewed => Accepted


Comment:

OK. Seems reasonable. Thanks.

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

Django

unread,
Dec 4, 2019, 3:28:44 AM12/4/19
to django-...@googlegroups.com
#30910: Add system check warning on duplicate check constraints.
--------------------------------------+------------------------------------

Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: closed

Component: Core (System checks) | Version: master
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

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

* status: new => closed
* resolution: => wontfix


Comment:

Reviewing again with Mariusz, we're not convinced the proposed solution is
sufficiently robust to capture more than the most naive cases. (There are
just too many ways of writing equivalent constraints.) As such, we're not
convinced of the value of the check. (Leaning towards a suspicion it would
actually be unhelpful on balance, which we've seen with other proposed
system checks at times.) Will close on that basis.

Possible to revisit if a way of ''reducing'' semantically equivalent
constraints turns up...


(c.f. [​https://github.com/django/django/pull/11973 Discussion on PR].)

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

Reply all
Reply to author
Forward
0 new messages