[Django] #33080: With Oracle migrating field that's FileField(null=True) to FileField(blank=True) changes column to NOT NULL (it shouldn't)

[Django]

#33080: With Oracle migrating field that's FileField(null=True) to
FileField(blank=True) changes column to NOT NULL (it shouldn't)
-------------------------------------+-------------------------------------
Reporter: Matt | Owner: nobody
Hoskins |
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Oracle stores empty strings as NULL values and Django has code to handle
that, but there is a bug.

Background: I just tried using django-silk with an Oracle database as the
back end and it was generating errors trying to safe a blank value in a
FileField that had blank=True set. The error was:
django.db.utils.IntegrityError: ORA-01400: cannot insert NULL into
("MISPORTAL"."SILK_REQUEST"."PROF_FILE")

As noted Django has code to deal with the fact Oracle stores empty strings
as NULL values (and sets char fields as NULL rather than NOT NULL if
blank=True), so it shouldn't happen. But looking through the migration
history I saw that the schema originally defined the field with null=True
and changed it to blank=True later, so there's an AlterField that comes
into play where the null attribute on the field has changed.

From a quick skim of the code I believe Django has code to ignore
migrations which change whether a column is nullable or not if
connection.features.interprets_empty_strings_as_nulls is True but only if
the field's get_internal_type() is "CharField" or "TextField" - that
obviously doesn't include "FileField". So although Django will correctly
create FileField columns where blank=True as "NULL" under Oracle, if they
were originally null=True and are later migrated to just be blank=True
then Django won't skip changing the column to "NOT NULL" and so you'll end
up with the column being NOT NULL and empty strings in that field will
result in the error as Oracle tries to save them as NULL values.

The bit which I think needs changing to fix this is in
db/backends/base/schema.py and is in _alter_column_null_sql - I think just
adding "FileField" to the list should do it. Having said that - I don't
know enough about Django's internals but it does seem a little strange
that the test in _alter_column_null_sql doesn't mirror the test that is in
_iter_column_sql of checking "(field.empty_strings_allowed and not
field.primary_key and
self.connection.features.interprets_empty_strings_as_nulls)" rather than
the specific type - although perhaps the test in _iter_column_sql got
changed in the past and _alter_column_null_sql got missed as being altered
to match it?

--
Ticket URL: <https://code.djangoproject.com/ticket/33080>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33080: Changing nullability for fields which allow empty string should not change
nullability on Oracle.
-------------------------------------+-------------------------------------
Reporter: Matt Hoskins | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle null | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* status: new => assigned
* owner: nobody => Mariusz Felisiak
* keywords: => oracle null
* has_patch: 0 => 1
* stage: Unreviewed => Accepted

Old description:

New description:

--

Comment:

Thanks for the report! As far as I'm aware `FilePathField`, `SlugField`,
`URLField`, `BinaryField`, and `FieldFile` are affected.

[https://github.com/django/django/pull/14822 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/33080#comment:1>

[Django]

#33080: Changing nullability for fields which allow empty string should not change
nullability on Oracle.
-------------------------------------+-------------------------------------
Reporter: Matt Hoskins | Owner: Mariusz
| Felisiak

Type: Bug | Status: closed

Component: Database layer | Version: 3.2
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: oracle null | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"338fc0e7f10f7ca781b059f2ff46e1a1a85c91f8" 338fc0e7]:
{{{
#!CommitTicketReference repository=""
revision="338fc0e7f10f7ca781b059f2ff46e1a1a85c91f8"
Fixed #33080 -- Preserved nullability of textual fields on Oracle.

Thanks Matt Hoskins for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33080#comment:2>