#10244 FileFields can't be set to NULL in the db

[oyvind....@gmail.com]

If a FileField with null=True is set to None, the db stores '' in the
db and not NULL as I would expect.

Also, if a FileField has both blank=True and null=True a ModelForm
without a file will save '' in the db, not sure if this is the desired
behaviour.

So the question is should the behaviour be as-is and if not is the
correct place to solve it in get_db_prep_value?

Example of code that this issue affects:

Model.objects.filter(filefield=u'') seems wrong as compared to
Model.objects.filter(filefield__isnull=True)

Model.objects.aggregate(Count('filefield')) i would expect this to
count objects with a file and not those without a file.

This may relate to other fields aswell, if a field has both blank=True
and null=True should it not store NULL in the db?

[Collin Grady]

It will only store NULL if you set it to None - if you leave the field
blank in a form or admin, there's still an empty string posted for
most string field types, so it stores that empty string.

--
Collin Grady

[Øyvind Saltvik]

The problem is, it is impossible to make a FileField None.

>>> from django.db import connection
>>> from app.models import Model
>>> instance = Model.objects.all()[0]
>>> instance.filefield = None
>>> instance.save()
>>> print connection.queries[-1]

On 20 Feb, 19:06, Collin Grady <col...@collingrady.com> wrote:
> It will only store NULL if you set it to None - if you leave the field
> blank in a form or admin, there's still an empty string posted for
> most string field types, so it stores that empty string.
>

> On Fri, Feb 20, 2009 at 4:57 AM, oyvind.salt...@gmail.com

[Øyvind Saltvik]

Attached new patch to ticket that should make the behaviour as
expected.

[Karen Tracey]

On Fri, Feb 20, 2009 at 3:21 PM, Øyvind Saltvik <oyvind....@gmail.com> wrote:

Attached new patch to ticket that should make the behaviour as
expected.

To what ticket?  There's no reference to a ticket in this (argh, top-posted) thread.

On 20 Feb, 20:15, Øyvind Saltvik <oyvind.salt...@gmail.com> wrote:
> The problem is, it is impossible to make a FileField None.
>
> >>> from django.db import connection
> >>> from app.models import Model
> >>> instance = Model.objects.all()[0]
> >>> instance.filefield = None
> >>> instance.save()
> >>> print connection.queries[-1]

Am I missing something?  This isn't very enlightening without the output.  Or are you telling me to go try that in my own shell?  Why not just include the output and spare me that work?

>
> On 20 Feb, 19:06, Collin Grady <col...@collingrady.com> wrote:
>
> > It will only store NULL if you set it to None - if you leave the field
> > blank in a form or admin, there's still an empty string posted for
> > most string field types, so it stores that empty string.
>

I have no idea what any of the 'its' in this (argh again, top-posted) reply referred to, making it pretty incomprehensible to me.  I think it (the reply) is trying to point out that for string fields in general the convention is "no input" gets translated to '' vs. NULL and you actually have to explicitly set values to None if you want the NULL showing up in the DB.

> > On Fri, Feb 20, 2009 at 4:57 AM, oyvind.salt...@gmail.com
>
> > <oyvind.salt...@gmail.com> wrote:
>
> > > If a FileField with null=True is set to None, the db stores '' in the
> > > db and not NULL as I would expect.
>
> > > Also, if a FileField has both blank=True and null=True a ModelForm
> > > without a file will save '' in the db, not sure if this is the desired
> > > behaviour.
>

I don't find it that unexpected.  No input specified translates to '', not NULL, for character fields.  It is true that you can force a character field to NULL if you specify None instead of leaving it out entirely, but this is discouraged, see:

http://docs.djangoproject.com/en/dev/ref/models/fields/#null

"Avoid using null on string-based fields such as CharField and TextField unless you have an excellent reason. If a string-based field has null=True, that means it has two possible values for "no data": NULL, and the empty string. In most cases, it's redundant to have two possible values for "no data;" Django convention is to use the empty string, not NULL."

Given that convention, I find it consistent that "no file" is represented by '' rather than NULL in database.  After all it is not the whole file that is stored the DB, just its name.  That the name for FileField=None is '', not NULL, seems a reasonable choice to me and consistent with the overall choice to use '' to represent "empty" for character data.
 

> > > So the question is should the behaviour be as-is and if not is the
> > > correct place to solve it in get_db_prep_value?
>

Regardless of what anyone thinks the behavior "should" be, whatever 1.0 in fact does is what needs to be maintained at this point, unless that behavior is broken to the point of unusability.  Introducing None->NULL now where in the past None->''  means people could face a situation where they have to check for both possibilities when looking for "no file" matches.  I don't see any good reason for introducing such backwards incompatibility.
 

> > > Example of code that this issue affects:
>
> > > Model.objects.filter(filefield=u'') seems wrong as compared to
> > > Model.objects.filter(filefield__isnull=True)
>

I don't find the first wrong, rather I find it consistent with the treatment of simple string fields.
 

> > > Model.objects.aggregate(Count('filefield')) i would expect this to
> > > count objects with a file and not those without a file.
>

This one is a little more awkward to write properly, but still not impossible:

Model.objects.exclude(filefield='').aggregate(Count('filefield'))

> > > This may relate to other fields aswell, if a field has both blank=True
> > > and null=True should it not store NULL in the db?
>

Not given the convention chosen long ago.  Mixing it up at this point would cause more confusion than clarity, I believe.

(And still after going backwards through the whole thread I don't see a ticket referenced?)

Karen

[Alex Gaynor]

The ticket is: http://code.djangoproject.com/ticket/10244

And I fluctuate on how I feel, OTOH you're right that it's always just been a string in the BD, OTOH at the python level we abstract very clearly that the DB is just holding a string.

Alex

--
"I disapprove of what you say, but I will defend to the death your right to say it." --Voltaire
"The people's good is the highest law."--Cicero

[Øyvind Saltvik]

Topic of the thread says ticket #10244.

On filefield being saved in the db as '' when saved from the form,
that makes perfect sense now.

But explicitly setting the field to None in python should store NULL
in the db.

On 20 Feb, 22:05, Karen Tracey <kmtra...@gmail.com> wrote:

[Karen Tracey]

On Fri, Feb 20, 2009 at 5:18 PM, Øyvind Saltvik <oyvind....@gmail.com> wrote:

Topic of the thread says ticket #10244.

Ah, so it is, I missed that.  Sorry.  I'm often blind to thread titles.
 

On filefield being saved in the db as '' when saved from the form,
that makes perfect sense now.

But explicitly setting the field to None in python should store NULL
in the db.

I think it's debatable, I can see the argument both ways.

I still haven't heard any convincing case that warrants breaking backwards compatibility at this point, though.  Right now, someone may have code that sets FileFields to None and uses a count of FileFields equal to empty string to count up "no file".  (Or counts number of files by counting not equal to ''.)  If we changed it to store NULL instead of empty string for this case, this existing code that counts number of "empty" files would break as soon as a new file was stored using the None->NULL mapping.  I don't see how we can change this at this point.

Karen

[Øyvind Saltvik]

I get the backwards compatibility issue, so if it cant be changed that
is understandable.

Also I simplified the issue about aggregate, I should have shown the
actual use case, to only show galleries of with videos that has been
converted.

Gallery.objects.annotate(video_count=Count('videos__flv_video')).filter
(video_count__gt=0) where videos is a m2m on gallery, so not being
able to have NULLs became a issue.

On 21 Feb, 00:43, Karen Tracey <kmtra...@gmail.com> wrote: