[Django] #28405: CICharField on a ModelFormSet doesn't catch unique constraint violations with different capitalization
Django
with different capitalization
--------------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
When `CICharField` (new in 1.11) with `unique=True` is used on a
`ModelFormSet`, values differing only by case are not reported as a
validation error. As the form appears to be valid, saving the form will
result in a database `IntegrityError`.
Will attach example test.
For example:
{{{
class MyModel(models.Model):
name = CICharField(max_length=100, unique=True)
MyModelFormset = modelformset_factory(MyModel, fields=['name'])
data = {
'form-TOTAL_FORMS': 2,
'form-INITIAL_FORMS': 0,
'form-0-name': 'frank',
'form-1-name': 'FRANK',
}
formset = MyModelFormSet(data)
}}}
Trying to save the formset, which reports no validation errors, results in
an exception:
{{{
Traceback (most recent call last):
File "/home/jon/devel/django/tests/postgres_tests/test_citext.py", line
59, in test_duplicate_formset_mixed_case
formset.save()
File "/home/jon/devel/django/django/forms/models.py", line 671, in save
return self.save_existing_objects(commit) +
self.save_new_objects(commit)
File "/home/jon/devel/django/django/forms/models.py", line 806, in
save_new_objects
self.new_objects.append(self.save_new(form, commit=commit))
File "/home/jon/devel/django/django/forms/models.py", line 647, in
save_new
return form.save(commit=commit)
File "/home/jon/devel/django/django/forms/models.py", line 457, in save
self.instance.save()
File "/home/jon/devel/django/django/db/models/base.py", line 711, in
save
force_update=force_update, update_fields=update_fields)
File "/home/jon/devel/django/django/db/models/base.py", line 741, in
save_base
updated = self._save_table(raw, cls, force_insert, force_update,
using, update_fields)
File "/home/jon/devel/django/django/db/models/base.py", line 825, in
_save_table
result = self._do_insert(cls._base_manager, using, fields, update_pk,
raw)
File "/home/jon/devel/django/django/db/models/base.py", line 863, in
_do_insert
using=using, raw=raw)
File "/home/jon/devel/django/django/db/models/manager.py", line 82, in
manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/home/jon/devel/django/django/db/models/query.py", line 1047, in
_insert
return query.get_compiler(using=using).execute_sql(return_id)
File "/home/jon/devel/django/django/db/models/sql/compiler.py", line
1202, in execute_sql
cursor.execute(sql, params)
File "/home/jon/devel/django/django/db/backends/utils.py", line 62, in
execute
return self.cursor.execute(sql, params)
File "/home/jon/devel/django/django/db/utils.py", line 89, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/home/jon/devel/django/django/db/backends/utils.py", line 62, in
execute
return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: duplicate key value violates unique
constraint "postgres_tests_citestmodel2_name_key"
DETAIL: Key (name)=(FRANK) already exists.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28405>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* Attachment "test.diff" added.
An example unit test
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
A related issue (case-insensitive collations on MySQL causing this
problem) is #23964.
--
Ticket URL: <https://code.djangoproject.com/ticket/28405#comment:1>
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
As a potential solution, what do others think about catching the
database's `IntegrityError` during `ModelForm.save()` to report unique
constraint violations in model forms? The database is already optimized
for robust checking of unique constraints, why repeat that code at the
application layer? It produces subtle differences and edge cases. This
solution could potentially deprecate the large amounts of Django code used
to check if a submitted form violates unique constraints and rely on the
basic database feature instead. All builtin datbase backends support
unique constraints. If we rely on the database, we wouldn't need to figure
out how the database is comparing rows (case sensitive vs case
insensitive), it'll tell us.
Something to note, this would mean that not all validation occurs before
the `ModelForm.save()` and that the `ModelForm.save()` can be unsuccessful
and could add errors to the form.
(Usual backward compatibility concerns apply.)
--
Ticket URL: <https://code.djangoproject.com/ticket/28405#comment:2>
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I think it'd be difficult to translate a database's exception into the
corresponding (translated) Django message.
--
Ticket URL: <https://code.djangoproject.com/ticket/28405#comment:3>
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
I feel like the underlying issue here is that the formset logic is not
relying on the database to determine unicity of values like a model form
does through `Model.validate_unique`.
Having `CICharField` and friends return a subclass of `str` that
implements `__eq__(self, other): self.casefold() == other.casefold()` as
suggested in #28405 would solve the issue for these fields but that not
for case-insensitive collations (#23964) and any other kind of non-
injective functional unique index. The same thing can be said about
[https://github.com/django/django/pull/7615 database level constraints].
--
Ticket URL: <https://code.djangoproject.com/ticket/28405#comment:4>
Django
with different capitalization
Reporter: Jon Dufresne | Owner: (none)
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => duplicate
Comment:
Marking as a duplicate of #23964 (see
[https://code.djangoproject.com/ticket/23964#comment:9 comment]).
--
Ticket URL: <https://code.djangoproject.com/ticket/28405#comment:5>