Opened 5 years ago

Closed 5 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 5 years ago.
Demonstration
tests-31410.diff (2.6 KB ) - added by Mariusz Felisiak 5 years ago.
Tests.

Download all attachments as: .zip

Change History (15)

by Marnanel Thurman, 5 years ago

Attachment: uniqueconstraint_bug.zip added

Demonstration

comment:1 by Marnanel Thurman, 5 years ago

Component: UncategorizedMigrations

comment:2 by Marnanel Thurman, 5 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 5 years ago

Component: MigrationsDatabase layer (models, ORM)
Easy pickings: set
Summary: makemigrations doesn't check that UniqueConstraint field names existAdd check for fields of UniqueConstraints.
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

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

by Mariusz Felisiak, 5 years ago

Attachment: tests-31410.diff added

Tests.

comment:4 by Rafael Arellano, 5 years ago

Owner: changed from nobody to Rafael Arellano
Status: newassigned

comment:5 by Jannah Mandwee, 5 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, 5 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, 5 years ago

Cc: Ian Foote added

comment:8 by Ian Foote, 5 years ago

CheckConstraint might have the same bug.

comment:9 by Mariusz Felisiak, 5 years ago

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

comment:10 by Hasan Ramezani, 5 years ago

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

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

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

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, 5 years ago

Patch needs improvement: unset

CheckConstraint checks removed from patch.

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

Resolution: fixed
Status: assignedclosed

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