Opened 7 years ago

Closed 6 years ago

#31410 closed Cleanup/optimization (fixed)

Add check for fields of UniqueConstraints.

Reported by: Marnanel Thurman Owned by: Hasan Ramezani
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Ian Foote Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Marnanel Thurman)

When a model gains a UniqueConstraint, makemigrations doesn't check that the fields named therein actually exist.

This is in contrast to the older unique_together syntax, which raises models.E012 if the fields don't exist.

In the attached demonstration, you'll need to uncomment "with_unique_together" in settings.py in order to show that unique_together raises E012.

Attachments (2)

uniqueconstraint_bug.zip​ (17.9 KB ) - added by Marnanel Thurman 7 years ago.
Demonstration
tests-31410.diff​ (2.6 KB ) - added by Mariusz Felisiak 7 years ago.
Tests.

Download all attachments as: .zip

Change History (15)

by Marnanel Thurman, 7 years ago

Attachment: uniqueconstraint_bug.zip​ added

Demonstration

comment:1 by Marnanel Thurman, 7 years ago

Component: Uncategorized → Migrations

comment:2 by Marnanel Thurman, 7 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 7 years ago

Component: Migrations → Database layer (models, ORM)
Easy pickings: set
Summary: makemigrations doesn't check that UniqueConstraint field names exist → Add check for fields of UniqueConstraints.
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Cleanup/optimization

Agreed. We can simply call cls._check_local_fields() for UniqueConstraint's fields. I attached tests.

by Mariusz Felisiak, 7 years ago

Attachment: tests-31410.diff​ added

Tests.

comment:4 by Rafael Arellano, 7 years ago

Owner: changed from nobody to Rafael Arellano
Status: new → assigned

comment:5 by Jannah Mandwee, 7 years ago

Hello Django Team,

My name is Jannah Mandwee, and I am working on my final project for my undergraduate software engineering class (here is the link to the assignment: ​https://web.eecs.umich.edu/~weimerw/481/hw6.html). I have to contribute to an open-source project and was advised to look through easy ticket pickings. I am wondering if it is possible to contribute to this ticket or if there is another ticket you believe would be a better fit for me. Thank you for your help.

in reply to:  5 comment:6 by Rafael Arellano, 7 years ago

Replying to Jannah Mandwee:

Hello Django Team,

My name is Jannah Mandwee, and I am working on my final project for my undergraduate software engineering class (here is the link to the assignment: ​https://web.eecs.umich.edu/~weimerw/481/hw6.html). I have to contribute to an open-source project and was advised to look through easy ticket pickings. I am wondering if it is possible to contribute to this ticket or if there is another ticket you believe would be a better fit for me. Thank you for your help.

Hi Jannah, I'm working in this ticket. You can consult this report: https://code.djangoproject.com/query?status=!closed&easy=1&stage=Accepted&order=priority there are all the tickets marked as easy.

comment:7 by Ian Foote, 7 years ago

Cc: Ian Foote added

comment:8 by Ian Foote, 7 years ago

CheckConstraint might have the same bug.

comment:9 by Mariusz Felisiak, 6 years ago

Summary: Add check for fields of UniqueConstraints. → Add check for fields of UniqueConstraints and CheckConstraint.

comment:10 by Hasan Ramezani, 6 years ago

Has patch: set
Owner: changed from Rafael Arellano to Hasan Ramezani
Last edited 6 years ago by Mariusz Felisiak (previous) (diff)

in reply to:  8 comment:11 by Mariusz Felisiak, 6 years ago

Patch needs improvement: set
Summary: Add check for fields of UniqueConstraints and CheckConstraint. → Add check for fields of UniqueConstraints.
Version: 3.0 → master

Replying to Ian Foote:

CheckConstraint might have the same bug.

After reconsideration I think we should limit this check only to UniqueConstraint's, it's not worth to add complex and error prone logic for CheckConstraint's, we can fail of custom expressions etc.

comment:12 by Hasan Ramezani, 6 years ago

Patch needs improvement: unset

CheckConstraint checks removed from patch.

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

Resolution: → fixed
Status: assigned → closed

In 3c7bf39:

Fixed #31410 -- Added system checks for invalid model field names in UniqueConstraint.

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