Form validation problem for model with ForeignKey having unique=True,blank=True,null=True

[nategriswold]

I'm having problems validating a form for the below models. Multiple null values in the below "Thing" model's "other" column seem to prevent the basic ModelForm from validating. This also happens for OneToOneFields of the same nature. Normal django db api functions and the database do not seem to have any problem. Basically, django forms are treating null as conflicting with null but the backend and db api are not. I don't think this is intended.

I also tried this in an older revision of django (r7477, using form_for_model) and did not have the problem there.

Thanks

class OtherThing(models.Model):
   pass

class Thing(models.Model):
   other = models.ForeignKey('OtherThing', null=True, blank=True, unique=True)

>>> import django
>>> django.get_version()
>>> from django.forms import ModelForm
>>> from test.models import *
>>>
>>> class ThingForm(ModelForm):
...   class Meta:
...     model = Thing
...
>>> Thing.objects.all()
[]
>>> ThingForm({}).save()
<Thing: Thing object>
>>> ThingForm({}).save()
Traceback (most recent call last):
 File "<console>", line 1, in ?
 File "/home/griswold/lib/python/django/forms/models.py", line 302, in save
   return save_instance(self, self.instance, self._meta.fields,
fail_message, commit)
 File "/home/griswold/lib/python/django/forms/models.py", line 36, in
save_instance
   raise ValueError("The %s could not be %s because the data didn't"
ValueError: The Thing could not be created because the data didn't validate.
>>>
>>> Thing.objects.create()
<Thing: Thing object>
>>> Thing.objects.create()
<Thing: Thing object>
>>> Thing.objects.all()
[<Thing: Thing object>, <Thing: Thing object>, <Thing: Thing object>]

mysql> show create table test_thing\G
*************************** 1. row ***************************
       Table: test_thing
Create Table: CREATE TABLE `test_thing` (
  `id` int(11) NOT NULL auto_increment,
  `other_id` int(11) default NULL,
  PRIMARY KEY  (`id`),
  KEY `test_thing_other_id` (`other_id`),
  CONSTRAINT `other_id_refs_id_52a1a3a0adf89f6` FOREIGN KEY (`other_id`) REFERENCES `test_otherthing` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1
1 row in set (0.00 sec)
}}}[[BR]]

[Alex Gaynor]

Ignore that patch, it doesn't work.

[Alex Gaynor]

Ok this should work

[paltman]

for what it is worth I have reviewed and tested this patch and can verify/validate that it is working as intended. I currently have my system patched, but would be nice to get it into trunk so that i don't have to remember to apply this patch in my different environments.

[SebastienWacquiez]

I just used the .2 patch to allow null value inside a "unique_together", it just work.

[smcoll]

i also confirm that this works, and would like to see it rolled into the code.

[Karen Tracey]

I do not understand the approach taken in the patch. Adding lookup_kwargs["%s__isnull" % field_name] = False to the database lookup that checks for uniqueness results in obviously not going to match anything queries such as:

SELECT (1) AS `a` FROM `model_forms_patient` WHERE (`model_forms_patient`.`special_id` IS NOT NULL AND `model_forms_patient`.`special_id` IS NULL)

or an unnecessary condition like:

SELECT (1) AS `a` FROM `model_forms_patient` WHERE (`model_forms_patient`.`special_id` IS NOT NULL AND `model_forms_patient`.`special_id` = 1 )

Why not simply avoid adding the field to those checked if it can be null and the provided value is empty? What am i missing here?

[Karen Tracey]

(In [9239]) Fixed #9039 -- Don't perform unique checks on NULL values, since NULL != NULL in SQL.

[Karen Tracey]

(In [9240]) [1.0.X] Fixed #9039 -- Don't perform unique checks on NULL values, since NULL != NULL in SQL.

Backport of [9239] from trunk.

[Erin Kelly]

I'm reopening this for further discussion because the test case fails in Oracle. The assumption that null doesn't count for uniqueness only holds when all the columns in the constraint are null. As a result, the form passes validation but then results in an IntegrityError when we try to save it.

Probably the ideal solution would be to allow the form to validate depending on the backend, so that this would work when possible and fail validation when not possible. But I don't think we currently do any backend-specific form validation, and I'm wary of what the implications of adding that might be -- especially in a multi-db scenario.

[Karen Tracey]

Ugh. Agreed that best would be if we could validate the form in a way that matched what the backend was going to enforce, but we don't seem to have anything like that in place and like you I'd be wary of moving in that direction. Could we just remove the 2nd save in the testcase to avoid triggering the error? We're left with a form that will validate when the acutaul DB save() is going to fail on at least one backend. Not ideal but more attractive to me than having the form refuse to validate values that some DBs find acceptable. And in the general case form validation doesn't guarantee that the DB save is actually going to work, so code should be prepared to deal with DB errors even after a successful validate()...right?

[Erin Kelly]

Yeah, that makes the most sense to me as well.

[Karen Tracey]

(In [9288]) Fixed #9039 take 2 -- Modified the new tests added in [9239] so they pass on Oracle.

[Karen Tracey]

(In [9290]) [1.0.X] Fixed #9039 take 2 -- Modified the new tests added in [9240] so they pass on Oracle.

Backport of [9288] from trunk, also updated svnmerge.py metadata.