Opened 11 years ago

Closed 5 years ago

Last modified 5 years ago

#702 closed New feature (fixed)

document that ManyToMany fields can't be in unique_together

Reported by: hugo Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords: model-validation
Cc: django@…, andre.miras+djangoproject@…, dannyman@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

From IRC:

  • hugo- any way to get a unique_together that works with ManyToMany fields?
  • adrian_h I don't believe so, at the moment
  • hugo- yeah. I had it working with some simple validator, until I realized that my validator can't know wether I am creating or updating (and if updating, which one I am updating) :-)
  • hugo- I _think_ it could be done with the same check you do in manipulators_validator_unique_together in core.meta for ManyToOne
  • hugo- or at least a very similar one
  • adrian_h If a model has unique_together = (('slug', 'sites'),), where sites is a manytomany field, would the validator check that the slug is unique per each individual site, or per the specific combination of the sites?
  • hugo- per each site
  • hugo- at least that's how you use it in reverse: you use sites.get_current() to get the curent site and then for example check get_current().get_flatfile_object() to find the flatfile for that url for that site
  • hugo- currently you can add flatfiles for the same site with the same URL and get a nice exception ...
  • adrian_h Good point
  • adrian_h This is worth a ticket

Change History (18)

comment:1 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by mrts

Component: Core frameworkValidators
milestone: 1.0 beta

This should be provided by model validation. As model validation is scheduled as "maybe" for 1.0, marking as 1.0 beta.

comment:3 Changed 8 years ago by anonymous

Keywords: model-validation added

comment:4 Changed 8 years ago by Jacob

milestone: 1.0 betapost-1.0

Model validation is getting pushed until post-1.0, so this is, too.

comment:5 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:6 Changed 8 years ago by lpetrov@…

any future plans for this ticket ?

comment:7 Changed 7 years ago by jedie

Cc: django@… added

comment:8 Changed 6 years ago by Andre Miras

Cc: andre.miras+djangoproject@… added

comment:9 Changed 6 years ago by Łukasz Rekucki

Severity: normalNormal
Type: defectNew feature

comment:10 Changed 5 years ago by dannyman@…

Cc: dannyman@… added
Easy pickings: unset
UI/UX: unset

comment:11 Changed 5 years ago by Kushagra Sinha

Owner: changed from nobody to Kushagra Sinha
Status: newassigned

comment:12 Changed 5 years ago by Preston Holmes

Component: ValidatorsDatabase layer (models, ORM)
Owner: changed from Kushagra Sinha to nobody
Status: assignednew
Version: SVN

This should be in the db/orm component

for reference an attempt at implementation would have to be made here:

django.db.models.base.Model._perform_unique_checks

If the implementation is not practical (this ticket closed as wontfix) a note of the limitation should be noted in the docs

comment:13 Changed 5 years ago by Carl Meyer

Component: Database layer (models, ORM)Documentation
Summary: make unique_together work with ManyToManydocument that ManyToMany fields can't be in unique_together

I don't think we should allow m2m fields to be listed in unique_together.

In practical terms, this is cross-model validation between two models (the main model and the m2m through model, which may be implicit). This is quite challenging and different from a normal unique_together, because the validation code for the main model runs before the m2m models have even been saved, so implementing this requires either listening to an m2m_changed signal, or putting the validation code onto the m2m through model. In a regular unique_together, there is an actual UNIQUE constraint created in the database covering the unique_together fields; with an m2m unique-together it could only be enforced by the model-validation code. IOW, the underlying implementation would be quite different from the implementation of a normal non-m2m unique_together, and those differences would be bound to leak up into application code.

Also, the meaning of listing an m2m field in a unique_together is not self-evident, as demonstrated by the IRC conversation in the description on this ticket. If "charfield" is unique_together with "m2mfield", does that mean charfield must be unique with respect to each individual value of m2mfield, or with respect to the combination of m2mfield values? Clearly in the FlatPage case the former is desired, but I can certainly imagine cases in which the latter would be desired.

Lastly, there's already a sort of precedent here - ManyToManyFields cannot be set as "unique" (presumably because it would result in similar semantic questions and implementation challenges). It seems pretty reasonable that a field type which can't be set as unique also can't be included in unique_together.

In light of all this complexity, I think validation of uniqueness related to m2m fields should be left up to the application developer, via signals or an explicit through model, rather than built in to the ORM. I'm changing the component here to documentation, and the summary to reflect that we need to document that m2m fields can't be included in unique_together.

comment:14 Changed 5 years ago by Dan Poirier

Owner: changed from nobody to Dan Poirier
Status: newassigned

comment:16 Changed 5 years ago by Dan Poirier

Owner: changed from Dan Poirier to nobody
Status: assignednew

comment:17 Changed 5 years ago by Tim Graham

Resolution: fixed
Status: newclosed

In [17314]:

Fixed #702 - Documented that ManyToMany fields can't be in unique_together; thanks poirier for the patch.

comment:18 Changed 5 years ago by Tim Graham

In [17315]:

[1.3.X] Fixed #702 - Documented that ManyToMany fields can't be in unique_together; thanks poirier for the patch.

Backport of r17314 from trunk.

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