Code

Opened 8 years ago

Closed 2 years ago

Last modified 2 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

Attachments (0)

Change History (18)

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

  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by mrts

  • Component changed from Core framework to Validators
  • milestone set to 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 6 years ago by anonymous

  • Keywords model-validation added

comment:4 Changed 6 years ago by jacob

  • milestone changed from 1.0 beta to post-1.0

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

comment:5 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:6 Changed 5 years ago by lpetrov@…

any future plans for this ticket ?

comment:7 Changed 5 years ago by jedie

  • Cc django@… added

comment:8 Changed 4 years ago by Andre

  • Cc andre.miras+djangoproject@… added

comment:9 Changed 3 years ago by lrekucki

  • Severity changed from normal to Normal
  • Type changed from defect to New feature

comment:10 Changed 3 years ago by dannyman@…

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

comment:11 Changed 3 years ago by j4nu5

  • Owner changed from nobody to j4nu5
  • Status changed from new to assigned

comment:12 Changed 3 years ago by ptone

  • Component changed from Validators to Database layer (models, ORM)
  • Owner changed from j4nu5 to nobody
  • Status changed from assigned to new
  • Version set to 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 3 years ago by carljm

  • Component changed from Database layer (models, ORM) to Documentation
  • Summary changed from make unique_together work with ManyToMany to document 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 2 years ago by poirier

  • Owner changed from nobody to poirier
  • Status changed from new to assigned

comment:16 Changed 2 years ago by poirier

  • Owner changed from poirier to nobody
  • Status changed from assigned to new

comment:17 Changed 2 years ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

In [17314]:

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

comment:18 Changed 2 years ago by timo

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.