Prevent Oracle from changing field.null to True

Fixed #17957 -- when using Oracle and character fields, the fields
were set null = True to ease the handling of empty strings. This
caused problems when using multiple databases from different vendors,
or when the character field happened to be also a primary key.

The handling was changed so that NOT NULL is not emitted on Oracle
even if field.null = False, and field.null is not touched otherwise.

Thanks to bhuztez for the report, ramiro for triaging & comments,
ikelly for the patch and alex for reviewing.

django/db/backends/creation.py

@@ -50,7 +50,13 @@ class BaseDatabaseCreation(object):
             # Make the definition (e.g. 'foo VARCHAR(30)') for this field.
             field_output = [style.SQL_FIELD(qn(f.column)),
                 style.SQL_COLTYPE(col_type)]
-            if not f.null:
+            # Oracle treats the empty string ('') as null, so coerce the null
+            # option whenever '' is a possible value.
+            null = f.null
+            if (f.empty_strings_allowed and not f.primary_key and
+                    self.connection.features.interprets_empty_strings_as_nulls):
+                null = True
+            if not null:
                 field_output.append(style.SQL_KEYWORD('NOT NULL'))
             if f.primary_key:
                 field_output.append(style.SQL_KEYWORD('PRIMARY KEY'))

django/db/models/fields/__init__.py

@@ -85,11 +85,6 @@ class Field(object):
         self.primary_key = primary_key
         self.max_length, self._unique = max_length, unique
         self.blank, self.null = blank, null
-        # Oracle treats the empty string ('') as null, so coerce the null
-        # option whenever '' is a possible value.
-        if (self.empty_strings_allowed and
-            connection.features.interprets_empty_strings_as_nulls):
-            self.null = True
         self.rel = rel
         self.default = default
         self.editable = editable

django/db/models/sql/query.py

@@ -1193,7 +1193,7 @@ class Query(object):
                             entry.negate()
                             self.where.add(entry, AND)
                             break
-                if field.null:
+                if self.is_nullable(field):
                     # In SQL NULL = anyvalue returns unknown, and NOT unknown
                     # is still unknown. However, in Python None = anyvalue is False
                     # (and not False is True...), and we want to return this Python's
@@ -1396,7 +1396,8 @@ class Query(object):
                                 opts, target)
 
                     alias = self.join((alias, table, from_col, to_col),
-                            exclusions=exclusions, nullable=field.null)
+                                      exclusions=exclusions,
+                                      nullable=self.is_nullable(field))
                     joins.append(alias)
                 else:
                     # Non-relation fields.
@@ -1946,6 +1947,24 @@ class Query(object):
         self.select = [(select_alias, select_col)]
         self.remove_inherited_models()
 
+    def is_nullable(self, field):
+        """
+        A helper to check if the given field should be treated as nullable.
+
+        Some backends treat '' as null and Django treats such fields as
+        nullable for those backends. In such situations field.null can be
+        False even if we should treat the field as nullable.
+        """
+        # We need to use DEFAULT_DB_ALIAS here, as QuerySet does not have
+        # (nor should it have) knowledge of which connection is going to be
+        # used. The proper fix would be to defer all decisions where
+        # is_nullable() is needed to the compiler stage, but that is not easy
+        # to do currently.
+        if ((connections[DEFAULT_DB_ALIAS].features.interprets_empty_strings_as_nulls)
+            and field.empty_strings_allowed):
+            return True
+        else:
+            return field.null
 
 def get_order_dir(field, default='ASC'):
     """

docs/ref/databases.txt

@@ -685,11 +685,11 @@ NULL and empty strings
 
 Django generally prefers to use the empty string ('') rather than
 NULL, but Oracle treats both identically. To get around this, the
-Oracle backend coerces the ``null=True`` option on fields that have
-the empty string as a possible value. When fetching from the database,
-it is assumed that a NULL value in one of these fields really means
-the empty string, and the data is silently converted to reflect this
-assumption.
+Oracle backend ignores an explicit ``null`` option on fields that
+have the empty string as a possible value and generates DDL as if
+``null=True``. When fetching from the database, it is assumed that
+a ``NULL`` value in one of these fields really means the empty
+string, and the data is silently converted to reflect this assumption.
 
 ``TextField`` limitations
 -------------------------

docs/ref/models/fields.txt

@@ -55,9 +55,8 @@ string, not ``NULL``.
 
 .. note::
 
-    When using the Oracle database backend, the ``null=True`` option will be
-    coerced for string-based fields that have the empty string as a possible
-    value, and the value ``NULL`` will be stored to denote the empty string.
+    When using the Oracle database backend, the value ``NULL`` will be stored to
+    denote the empty string regardless of this attribute.
 
 If you want to accept :attr:`~Field.null` values with :class:`BooleanField`,
 use :class:`NullBooleanField` instead.

tests/regressiontests/queries/tests.py

@@ -1930,3 +1930,25 @@ class NullInExcludeTest(TestCase):
         self.assertQuerysetEqual(
             NullableName.objects.exclude(name__in=[None]),
             ['i1'], attrgetter('name'))
+
+class EmptyStringsAsNullTest(TestCase):
+    """
+    Test that filtering on non-null character fields works as expected.
+    The reason for these tests is that Oracle treats '' as NULL, and this
+    can cause problems in query construction. Refs #17957.
+    """
+
+    def setUp(self):
+        self.nc = NamedCategory.objects.create(name='')
+
+    def test_direct_exclude(self):
+        self.assertQuerysetEqual(
+            NamedCategory.objects.exclude(name__in=['nonexisting']),
+            [self.nc.pk], attrgetter('pk')
+        )
+
+    def test_joined_exclude(self):
+        self.assertQuerysetEqual(
+            DumbCategory.objects.exclude(namedcategory__name__in=['nonexisting']),
+            [self.nc.pk], attrgetter('pk')
+        )