FileFields can't be set to NULL in the db

[oyvind]

Saving FileFields with a none value sets the field to a empty string in the db and not NULL as it should.

[oyvind]

Patch does not seem to work quite like I expected. So the problem persists.

[oyvind]

Example to test this issue:

                
class File(models.Model):

    file = models.FileField(null=True, upload_to='files') 

from models import File

f = File()
f.file = None
f.save()

from django.db import connection
print connection.queries

[oyvind]

Patch that solves this issue, FieldFile's name can now be None also "is" and "==" is not the same

[Alex Gaynor]

needs to be decided whether the CharField behavior should be used throughout, or NULL where explicitly requested

[oyvind]

Meade FieldFile set name to u on delete, made test show filefields and charfields act the same

[Karen Tracey]

Dev list discussion: ​http://groups.google.com/group/django-developers/browse_thread/thread/cf37abc95e49f0e/

Also, #10749 reported surprise at searching for None not working when looking for empty ImageFields. I still think changing this now breaks backwards-compatibility, though.

[Aymeric Augustin]

I see two possible solutions:

• 1. document that FileField ignores the null keyword argument; this is the most backwards compatible solution;
• 2. make FileField behave exactly like CharField wrt. null; this is the most consistent solution.

Option 2. will be backwards incompatible only for people who set null=True on a FileField. The docs discourage that heavily, and it doesn't behave as expected, actually it doesn't even do anything, so there isn't much reason to have such code. That's why I think the change would be acceptable.

I have a slight preference for option 2, which is better in the long term.

[siroky@…]

Hi! What is the status of this ticket? I see it is 5 years old ticket and the problem is still present at least in 1.5.1.

[Aymeric Augustin]

It's waiting for someone to write a patch.

[siroky@…]

None/NULL for FileField (v1.5.1)

[siroky@…]

How about the filefield_null.patch? It follows solution 2.

[Claude Paroz]

Thanks, it's a first step :-) However, tests are crucial with such a change. That will be the next step. See FileFieldTestsin tests/model_fields/tests.py.

[anonymous]

How can I run the FileFieldTests without the whole suite?

[Tim Graham]

See ​https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#running-only-some-of-the-tests

[siroky@…]

I bumped into a problem. If instance.null_file_field is None then methods like save() will not work (AttributeError: 'NoneType' object has no attribute 'save'). I'm thinking about 2 solutions:
1) Make the descriptor return value comparable to None (via __eq__). This is not very clean because the best practise is to use operator is (is_none = x is None).
2) Keep the empty string ("") on the Python side as a representation of file's database NULL. Not very consistent but it has better backward compatibility if someone already uses the string comparison (has_no_file = filefield.name == "").

Any suggestions?

[Claude Paroz]

#25528 was a duplicate.

[Raffaele Salmaso]

Hi all, what is the status of this ticket?
FileField behaviour is not really clear right now and flake8-django just made a change to allow FileField(null=True) (see ​https://github.com/rocioar/flake8-django/issues/82), which seems wrong to me.

I'm for option 2 make FileField behave exactly like CharField wrt. null.

[dennisvang]

From ten years ago:

... only for people who set null=True on a FileField. The docs discourage that heavily ...

As far as I can see, there is no mention of null, at all, in the ​current documentation for FileField, nor is FileField mentioned in the ​documentation for field option null.

Yet Django's FileField behavior is still plainly inconsistent, compared to other fields, when it comes to null=True.

A short summary:

We can specify null=True, and we can assign None to the field, but this is stored as a FieldFile with name=None (initially) on the python side, and it is stored as an empty string '' in the database. After e.g. refresh_from_db, the FieldFile.name will also be an empty string, instead of None.

Basically, the following snippet passes, where my_filefield is a FileField with null=True:

obj = MyModel.objects.create(my_filefield=None)
assert not MyModel.objects.filter(my_filefield__isnull=True).exists()
assert obj.my_filefield is not None
assert isinstance(obj.my_filefield, FieldFile)
assert obj.my_filefield.name is None
obj.refresh_from_db()
assert obj.my_filefield.name is not None
assert obj.my_filefield.name == ''

Some resulting inconveniences:

• MyModel.objects.create(my_filefield=None).my_filefield is not None
• my_model_instance.my_filefield is always "truthy", so we need to check e.g. my_filefield.name (​example)
• we cannot use the __isnull field lookup, but have to check for equality with the empty string (my_filefield=''), or even my_filefield__in=['', None] (​example)
• FileField(null=True, unique=True) is broken, because the database entry is '' instead of null, so we get a unique-constraint violation if we try to create another instance with None

The latter is basically what is described in the ​model field reference:

... One exception is when a CharField has both unique=True and blank=True set. In this situation, null=True is required to avoid unique constraint violations when saving multiple objects with blank values.

So, currently, we cannot make a FileField unique and optional at the same time, in the database.

This also breaks e.g. get_or_create for FileField.

Why not make a backward incompatible change for the next major release, for the sake of consistency?

[Ondrej Pudiš]

As I understand the issue, if the file field is specified as nullable, it should accept None and in that case, store NULL on the database level instead of an empty string. In fact, it shouldn't even create an instance of FieldFile when creating a new NullableDocument object.

The tests listed below are broken now but the patch should fix all of them giving a solution to all the issues enumerated in the comment above.

def test_create_empty(self):
    # when I create a new document with no file provided
    d = NullableDocument.objects.create(myfile=None)
    # I expect that the attribute itself is None
    self.assertIs(d.myfile, None)
    # I expect that I can filter the documents and find the one
    query = NullableDocument.objects.filter(myfile__isnull=True).exists()
    self.assertTrue(query)
    # I expect that the object remains None even after refreshing from the DB
    d.refresh_from_db()
    self.assertIs(d.myfile, None)

def test_create_empty_multiple(self):
    # when the files are expected to be unique but nullable, I expect that I'm
    # allowed to create multiple records
    NullableDocument.objects.create(myfile=None)
    NullableDocument.objects.create(myfile=None)
    # and both are created
    query = NullableDocument.objects.filter(myfile__isnull=True).count()
    self.assertEqual(query, 2)

def test_create_empty_on_not_null(self):
    # when I try to store an empty file on non-nullable model, I expect to get an
    # integrity error
    with self.assertRaises(IntegrityError):
        Document.objects.create(myfile=None)

[Ondrej Pudiš]

My proposed patch is to not create an instance of FieldFile when the file is None. However, this is a significant breaking change as people might be accessing the FileField and checking for name or path being an empty string.

Many tests would need to be adjusted to this new logic.

As suggested earlier, another possibility would be to store None on the DB level while interpreting it as an empty FieldFile on the application level. This cloud introduce more confusion to the whole mechanics of the files storing.

I believe that the first approach with not creating the FieldFile object when the file doesn't actually exists is a cleaner solution which is consistent with other types of database fields.