Opened 16 years ago
Last modified 15 months ago
#10244 assigned Bug
FileFields can't be set to NULL in the db — at Version 18
Reported by: | oyvind | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.0 |
Severity: | Normal | Keywords: | filefield NULL empty |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Saving FileFields with a none value sets the field to a empty string in the db and not NULL as it should.
Change History (21)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
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
by , 16 years ago
Attachment: | filefield_null.diff added |
---|
Patch that solves this issue, FieldFile's name can now be None also "is" and "==" is not the same
by , 16 years ago
Attachment: | null-filefield.diff added |
---|
comment:3 by , 16 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
needs to be decided whether the CharField behavior should be used throughout, or NULL where explicitly requested
by , 16 years ago
Attachment: | null-filefield-2.diff added |
---|
Meade FieldFile set name to u on delete, made test show filefields and charfields act the same
comment:4 by , 16 years ago
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.
comment:5 by , 16 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:7 by , 13 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
I see two possible solutions:
- 1. document that
FileField
ignores thenull
keyword argument; this is the most backwards compatible solution; - 2. make
FileField
behave exactly likeCharField
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.
comment:8 by , 11 years ago
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.
comment:11 by , 11 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Thanks, it's a first step :-) However, tests are crucial with such a change. That will be the next step. See FileFieldTests
in tests/model_fields/tests.py
.
comment:13 by , 11 years ago
comment:14 by , 11 years ago
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?
comment:17 by , 4 years ago
Description: | modified (diff) |
---|
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
.
comment:18 by , 4 years ago
Description: | modified (diff) |
---|
Patch does not seem to work quite like I expected. So the problem persists.