Changes between Initial Version and Version 1 of Ticket #33080
- Timestamp:
- Sep 1, 2021, 6:33:42 AM (3 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #33080
- Property Has patch set
- Property Keywords oracle null added
- Property Owner changed from to
- Property Triage Stage Unreviewed → Accepted
- Property Status new → assigned
- Property Summary With Oracle migrating field that's FileField(null=True) to FileField(blank=True) changes column to NOT NULL (it shouldn't) → Changing nullability for fields which allow empty string should not change nullability on Oracle.
-
Ticket #33080 – Description
initial v1 1 1 Oracle stores empty strings as NULL values and Django has code to handle that, but there is a bug. 2 2 3 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")3 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")` 4 4 5 5 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. 6 6 7 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.7 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. 8 8 9 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_sqlgot missed as being altered to match it?9 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?