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.

3 files changed, 28 insertions, 8 deletions

diff --git a/django/db/backends/creation.py b/django/db/backends/creation.py
index 0cd055866a..75afe926fa 100644
--- a/django/db/backends/creation.py
+++ b/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'))
diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
index dc4495390a..5a5e6cc8c3 100644
--- a/django/db/models/fields/__init__.py
+++ b/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
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index cf527b18b0..6111a88118 100644
--- a/django/db/models/sql/query.py
+++ b/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'):
     """