Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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.
  • Loading branch information...
commit 584e2c03376895aeb0404cc1fcc1ad24dfdbc58e 1 parent e75bd7e
@akaariai akaariai authored
View
8 django/db/backends/creation.py
@@ -50,7 +50,13 @@ def sql_create_model(self, model, style, known_models=set()):
# 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'))
View
5 django/db/models/fields/__init__.py
@@ -85,11 +85,6 @@ def __init__(self, verbose_name=None, name=None, primary_key=False,
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
View
23 django/db/models/sql/query.py
@@ -1193,7 +1193,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
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 @@ def setup_joins(self, names, opts, alias, dupe_multis, allow_many=True,
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 @@ def set_start(self, start):
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'):
"""
View
10 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
-------------------------
View
5 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.
View
22 tests/regressiontests/queries/tests.py
@@ -1930,3 +1930,25 @@ def test_col_not_in_list_containing_null(self):
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')
+ )
Please sign in to comment.
Something went wrong with that request. Please try again.