#4562 closed (wontfix)
unique_together uses 'iexact' instead of 'exact'
Reported by: | Owned by: | Jacob | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Keywords: | ||
Cc: | v.oostveen@…, bronger@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
The unique_together flag promises unique entries in a database. However the manipulator deploys case insensitive checks with "iexact" instead of "exact". The attached patch enforces the correct uniqueness check with "exact".
Attachments (2)
Change History (18)
by , 18 years ago
Attachment: | django_unique_together_exact.diff added |
---|
comment:1 by , 17 years ago
Component: | Uncategorized → Admin interface |
---|---|
Owner: | changed from | to
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 17 years ago
I just applied this patch to svn HEAD on one of my production servers last night. It fixed the problem and didn't seem to have any side-effects. Is anyone interested in fixing it?
comment:3 by , 17 years ago
Bug #5859 depends on this, it would seem, so it's probably worth fixing.
comment:4 by , 17 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:5 by , 17 years ago
Patch needs improvement: | set |
---|
Allowing instances which only differ by case will currently raise an exception with mysql (with case-insensitive collation, which is the default). See attached tests.
follow-up: 7 comment:6 by , 17 years ago
It seems this is another bug. Queries like name = 'John'
should be case sensitive, at least that is what Django expects. The solution whould be to use the binary
keyword in mysql, like this:
select from app_users where binary name = 'John'
But this should probably discussed in a separate ticket.
comment:7 by , 17 years ago
Or modify the backend driver for mysql to use the case-sensitive collation
comment:8 by , 17 years ago
Needs documentation: | set |
---|
FYI: this is also related to ticket #6523 which hits the same problem specifically with PostgreSQL 8.3
Any reason still open why it cannot be applied ? It seems to me that there is no need for a better patch:
Cause i feel strongly that "just because MySQL doesn't do this" is a very weak argument for needing a better patch. That is MySQL's fault not Django's and Django should not hot-patch it's code to fix problems with MySQL, the same for any other DB that is not case-sensitive per default. Thus there should be a seperate ticket for MySQL that applies the case-sensitive collation to every table or uses the binary keyword as pointed out above by Bastian.
But i would like to raise some thoughts about the potential problems that may arise from making uniques case-sensitive...
I can imagine people that are now accustomed to having Person(first_name="John",last_name="Doe") reject Person(first_name="JOHN",last_name="DOE") where unique_together = ('first_name', 'last_name') So maybe this should be done with some careful changes to the documentation outlining case sensitivity. Possibly introducing a new parameter which gives the user a way to change how matching should be done. As normally per default you would want to have case-sensitive matching but maybe sometimes (for instance with names) you would want the fields to be case-sensitive but the matching to be case insensitive. In other words normally use exact and give the user an option to specify when to use iexact... (But this is probably more of an after-thought)
Now I'm all in favor of pushing this patch in Django and then forcing all database backend to make sure the database uses case-sensitive behavior, because i feel that the reverse case which is now happening is far worse then the above. Especially as this is currently handled ambiguously:
>>> Test.objects.all() [<Test: john doe>] >>> foo=Test(first_name="JOHN", last_name="DOE") >>> foo.save() >>> Test.objects.all() [<Test: john doe>, <Test: JOHN DOE>]
While doing the same from the admin pages will raise ValidationError:
Test with this first name already exists for the given last name.
And after saving the object from command line or view the admin will be a little angry as it now violates the case_insensitive_unique_together constrained:
MultipleObjectsReturned at /admin/uniquetogether/test/2/ get() returned more than one Test -- it returned 2! Lookup parameters were {'last_name__iexact': u'DOE', 'first_name__iexact': u'JOHN'}
Sorry for the long comment this time :)
And enabled the needs_documentation as i feel that this change does require some documentation, either in the normal django docs or otherwise at least in the BackwardsIncompatibleChanges list.
comment:9 by , 17 years ago
Cc: | added |
---|
comment:10 by , 17 years ago
By the way, should not the manipulator_validator_unique_for_date() function be changed similarly?
Too bad it is not fixed yet, cause having neither this ticket now 6523 one fixed renders unique_together options utilizing foreign keys completely unusable on PG8.3.
comment:11 by , 17 years ago
milestone: | → 1.0 |
---|
comment:12 by , 16 years ago
Owner: | changed from | to
---|
comment:13 by , 16 years ago
Cc: | added |
---|
comment:14 by , 16 years ago
Owner: | changed from | to
---|
comment:15 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Manipulators are gone, so this is wontfix. However, this is part of a bigger problem that needs addressing; see #7789, for one.
See #4778 for an example of what the current behaviour breaks.
However, I'm not sure if this'll get much attention due to manipulators being on the "way out". Marking as DDR to confirm if it's worth fixing.