Fixed #4136 -- Made ModelForm save empty values for nullable CharFields as NULL.

Previously, empty values were saved as strings.

9 files changed, 66 insertions, 18 deletions

diff --git a/django/db/models/base.py b/django/db/models/base.py
index 52a96c3ffe..2444c22b01 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -13,8 +13,8 @@ from django.core.exceptions import (
     ObjectDoesNotExist, ValidationError,
 )
 from django.db import (
-    DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY, DatabaseError, connections,
-    router, transaction,
+    DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY, DatabaseError, connection,
+    connections, router, transaction,
 )
 from django.db.models import signals
 from django.db.models.constants import LOOKUP_SEP
@@ -1087,7 +1087,9 @@ class Model(six.with_metaclass(ModelBase)):
             for field_name in unique_check:
                 f = self._meta.get_field(field_name)
                 lookup_value = getattr(self, f.attname)
-                if lookup_value is None:
+                # TODO: Handle multiple backends with different feature flags.
+                if (lookup_value is None or
+                        (lookup_value == '' and connection.features.interprets_empty_strings_as_nulls)):
                     # no value, skip the lookup
                     continue
                 if f.primary_key and not self._state.adding:
diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
index 76e19eb6e9..4fdba3de81 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -1086,6 +1086,9 @@ class CharField(Field):
         # will be validated twice. This is considered acceptable since we want
         # the value in the form field (to pass into widget for example).
         defaults = {'max_length': self.max_length}
+        # TODO: Handle multiple backends with different feature flags.
+        if self.null and not connection.features.interprets_empty_strings_as_nulls:
+            defaults['empty_value'] = None
         defaults.update(kwargs)
         return super(CharField, self).formfield(**defaults)
 
diff --git a/django/forms/fields.py b/django/forms/fields.py
index c50cd164ed..a20caca870 100644
--- a/django/forms/fields.py
+++ b/django/forms/fields.py
@@ -214,10 +214,11 @@ class Field(object):
 
 
 class CharField(Field):
-    def __init__(self, max_length=None, min_length=None, strip=True, *args, **kwargs):
+    def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs):
         self.max_length = max_length
         self.min_length = min_length
         self.strip = strip
+        self.empty_value = empty_value
         super(CharField, self).__init__(*args, **kwargs)
         if min_length is not None:
             self.validators.append(validators.MinLengthValidator(int(min_length)))
@@ -227,7 +228,7 @@ class CharField(Field):
     def to_python(self, value):
         "Returns a Unicode object."
         if value in self.empty_values:
-            return ''
+            return self.empty_value
         value = force_text(value)
         if self.strip:
             value = value.strip()
diff --git a/docs/ref/forms/fields.txt b/docs/ref/forms/fields.txt
index 7144a46a9d..0a5703c46e 100644
--- a/docs/ref/forms/fields.txt
+++ b/docs/ref/forms/fields.txt
@@ -361,7 +361,7 @@ For each field, we describe the default widget used if you don't specify
 .. class:: CharField(**kwargs)
 
     * Default widget: :class:`TextInput`
-    * Empty value: ``''`` (an empty string)
+    * Empty value: Whatever you've given as :attr:`empty_value`.
     * Normalizes to: A Unicode object.
     * Validates ``max_length`` or ``min_length``, if they are provided.
       Otherwise, all inputs are valid.
@@ -380,6 +380,12 @@ For each field, we describe the default widget used if you don't specify
        If ``True`` (default), the value will be stripped of leading and
        trailing whitespace.
 
+    .. attribute:: empty_value
+
+       .. versionadded:: 1.11
+
+       The value to use to represent "empty". Defaults to an empty string.
+
 ``ChoiceField``
 ---------------
 
diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt
index a56c445c31..2435201db8 100644
--- a/docs/ref/models/fields.txt
+++ b/docs/ref/models/fields.txt
@@ -43,11 +43,13 @@ If ``True``, Django will store empty values as ``NULL`` in the database. Default
 is ``False``.
 
 Avoid using :attr:`~Field.null` on string-based fields such as
-:class:`CharField` and :class:`TextField` because empty string values will
-always be stored as empty strings, not as ``NULL``. If a string-based field has
+:class:`CharField` and :class:`TextField`. If a string-based field has
 ``null=True``, that means it has two possible values for "no data": ``NULL``,
 and the empty string. In most cases, it's redundant to have two possible values
-for "no data;" the Django convention is to use the empty string, not ``NULL``.
+for "no data;" the Django convention is to use the empty string, not
+``NULL``. One exception is when a :class:`CharField` has both ``unique=True``
+and ``blank=True`` set. In this situation, ``null=True`` is required to avoid
+unique constraint violations when saving multiple objects with blank values.
 
 For both string-based and non-string-based fields, you will also need to
 set ``blank=True`` if you wish to permit empty values in forms, as the
diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt
index d2e3e74fdd..709c4052f1 100644
--- a/docs/releases/1.11.txt
+++ b/docs/releases/1.11.txt
@@ -154,7 +154,8 @@ File Uploads
 Forms
 ~~~~~
 
-* ...
+* The new :attr:`CharField.empty_value <django.forms.CharField.empty_value>`
+  attribute allows specifying the Python value to use to represent "empty".
 
 Generic Views
 ~~~~~~~~~~~~~
@@ -258,6 +259,13 @@ Miscellaneous
   displays the related object's ID. Remove the ``_id`` suffix if you want the
   old behavior of the string representation of the object.
 
+* In model forms, :class:`~django.db.models.CharField` with ``null=True`` now
+  saves ``NULL`` for blank values instead of empty strings.
+
+* On Oracle, :meth:`Model.validate_unique()
+  <django.db.models.Model.validate_unique>` no longer checks empty strings for
+  uniqueness as the database interprets the value as ``NULL``.
+
 .. _deprecated-features-1.11:
 
 Features deprecated in 1.11
diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt
index 938516050a..9dcb2e3e53 100644
--- a/docs/topics/forms/modelforms.txt
+++ b/docs/topics/forms/modelforms.txt
@@ -66,7 +66,9 @@ Model field                         Form field
 
 :class:`CharField`                  :class:`~django.forms.CharField` with
                                     ``max_length`` set to the model field's
-                                    ``max_length``
+                                    ``max_length`` and
+                                    :attr:`~django.forms.CharField.empty_value`
+                                    set to ``None`` if ``null=True``.
 
 :class:`CommaSeparatedIntegerField` :class:`~django.forms.CharField`
 
diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py
index 2c3ff26cbe..ec39e5ad1f 100644
--- a/tests/model_forms/models.py
+++ b/tests/model_forms/models.py
@@ -483,3 +483,7 @@ class StrictAssignmentAll(models.Model):
 class Award(models.Model):
     name = models.CharField(max_length=30)
     character = models.ForeignKey(Character, models.SET_NULL, blank=False, null=True)
+
+
+class NullableUniqueCharFieldModel(models.Model):
+    codename = models.CharField(max_length=50, blank=True, null=True, unique=True)
diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py
index 78e7399637..297ccaae3d 100644
--- a/tests/model_forms/tests.py
+++ b/tests/model_forms/tests.py
@@ -28,9 +28,10 @@ from .models import (
     CustomErrorMessage, CustomFF, CustomFieldForExclusionModel, DateTimePost,
     DerivedBook, DerivedPost, Document, ExplicitPK, FilePathModel,
     FlexibleDatePost, Homepage, ImprovedArticle, ImprovedArticleWithParentLink,
-    Inventory, Person, Photo, Post, Price, Product, Publication,
-    PublicationDefaults, StrictAssignmentAll, StrictAssignmentFieldSpecific,
-    Student, StumpJoke, TextFile, Triple, Writer, WriterProfile, test_images,
+    Inventory, NullableUniqueCharFieldModel, Person, Photo, Post, Price,
+    Product, Publication, PublicationDefaults, StrictAssignmentAll,
+    StrictAssignmentFieldSpecific, Student, StumpJoke, TextFile, Triple,
+    Writer, WriterProfile, test_images,
 )
 
 if test_images:
@@ -270,6 +271,21 @@ class ModelFormBaseTest(TestCase):
         obj = form.save()
         self.assertEqual(obj.name, '')
 
+    def test_save_blank_null_unique_charfield_saves_null(self):
+        form_class = modelform_factory(model=NullableUniqueCharFieldModel, fields=['codename'])
+        empty_value = '' if connection.features.interprets_empty_strings_as_nulls else None
+
+        form = form_class(data={'codename': ''})
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(form.instance.codename, empty_value)
+
+        # Save a second form to verify there isn't a unique constraint violation.
+        form = form_class(data={'codename': ''})
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(form.instance.codename, empty_value)
+
     def test_missing_fields_attribute(self):
         message = (
             "Creating a ModelForm without either the 'fields' attribute "
@@ -800,10 +816,14 @@ class UniqueTest(TestCase):
         form.save()
         form = ExplicitPKForm({'key': 'key1', 'desc': ''})
         self.assertFalse(form.is_valid())
-        self.assertEqual(len(form.errors), 3)
-        self.assertEqual(form.errors['__all__'], ['Explicit pk with this Key and Desc already exists.'])
-        self.assertEqual(form.errors['desc'], ['Explicit pk with this Desc already exists.'])
-        self.assertEqual(form.errors['key'], ['Explicit pk with this Key already exists.'])
+        if connection.features.interprets_empty_strings_as_nulls:
+            self.assertEqual(len(form.errors), 1)
+            self.assertEqual(form.errors['key'], ['Explicit pk with this Key already exists.'])
+        else:
+            self.assertEqual(len(form.errors), 3)
+            self.assertEqual(form.errors['__all__'], ['Explicit pk with this Key and Desc already exists.'])
+            self.assertEqual(form.errors['desc'], ['Explicit pk with this Desc already exists.'])
+            self.assertEqual(form.errors['key'], ['Explicit pk with this Key already exists.'])
 
     def test_unique_for_date(self):
         p = Post.objects.create(