Ticket #15124: BooleanField should not use False as default (unless provided)

[Andrew Badr]

Models with a BooleanField are instantiated with that field's value
set to False if no default or initial value is provided. Instead, like
most other fields, the field's initial value should be set to None.
This None should be left uncoerced when attempting to save the
instance, so attempting to save should result in a database error for
trying to set null on a non-null field.

Although this change is backwards incompatible, that depends how far
backwards you go. Apparently the behavior was as I think it should be
at the time #2855 was created. The facts that (1) the behavior has
changed, and (2) #2855 was wontfix'd together make a strong case that
this should be considered a bug, and therefore should be changeable
immediately despite being backwards incompatible.

Links:
http://code.djangoproject.com/ticket/15124
http://code.djangoproject.com/ticket/2855

[Carl Meyer]

This seems right to me. Does anyone with more history on BooleanField
know of any reason why the "wontfix" resolution on #2855 was incorrect
and the current default-to-False behavior is correct?

Carl

[Chris Beaven]

For good or bad, in Django a BooleanField is only ever supposed to be True or False.

A default of False seems the logical equivalent to the default of '' on a not-null CharField.

If you want a nullable boolean, you should use the separate NullBooleanField.

[Karen Tracey]

Except a nullable boolean isn't what's being asked for. Rather, a BooleanField that raises an error on an attempt to save an instance that has no value set is what's being asked for. The quiet always defaulting to False does seem rather odd to me as well.

It was added here:

http://code.djangoproject.com/changeset/8050

I wonder if anyone can remember what the "interesting failures in subtle ways on MySQL and SQLite" were? I'd hesitate to remove it without a better understanding of why it was put in place, even though it does seem odd to me.

Does removing it cause any test failures?

Karen

[Andrew Badr]

> Does removing it cause any test failures?
>
> Karen

Yes, one, on sqlite:

Test:
ERROR: test_issue_6755
(regressiontests.model_inheritance_regress.tests.ModelInheritanceTest)

Exception:
IntegrityError: model_inheritance_regress_restaurant.serves_hot_dogs
may not be NULL

This is the desired behavior. We just don't know whether they serve
hot dogs. I've been there once but it was a long time ago.

#6755 seems unrelated to this issue ("Model Inheritance doesn't work
in the admin"). Haven't tested on other databases.

Andrew

[Russell Keith-Magee]

I've just run the suite under Postgres with the default removed, and I
don't see any errors, except for the #6755 error noted by Andrew.
However, that test appears to just be written to rely on the default
value, rather than requiring the default value.

I've also run the full test suite under MySQL/MyISAM [1] with the same result.

I don't remember what the "interesting failures" were -- but based on
the timestamp, this would have been something Malcolm was cleaning up
just prior to the v1.0 release. My guess would be that the
'interesting failures' stem from MySQL's slightly weird handling of
booleans -- while I don't know the specifics, it's not hard to imagine
that somewhere in the combination of interpretations and translations
of NULL, None, '0', 0 and False, something could go wrong. However,
we've significantly improved the handling of booleans in the MySQL
backend since r8050 landed, so it's possible the problems Malcolm was
referring to have been inadvertently fixed.

However, since Malcolm didn't include a test case, I suppose we'll
never know for certain.

So - working on the assumption that the problem doesn't exist anymore
(which based on Django's test suite, appears to be the case), what is
the right behavior?

I agree that the current behavior is strange, and the original
behavior preferred by #2855 seems right to me.

If we decide to revert the default, it's probably worth mentioning it
in the release notes -- even though a bug fix isn't technically a
backwards incompatibility, this issue is big enough a change that I
can see it causing confusion. For that reason, I'm also inclined to
suggest that we *shouldn't* backport the change.

Yours,
Russ Magee %-)
----

[1] A neat trick I've discovered: You can run the full MySQL test
suite a lot faster if you use
./runtest.py --settings=mysql --pair=bug639

bug639 is a simple, one test case bug, so the --pair is effectively
just "run every single test, but run each test in an isolated
environment". As a result, db flushes are a lot cheaper, so the suite
runs a lot faster. Error reporting is a lot more verbose, but that's a
small price to pay.

[Chris Beaven]

On Friday, January 21, 2011 12:35:58 PM UTC+13, Karen Tracey wrote:

Rather, a BooleanField that raises an error on an attempt to save an instance that has no value set is what's being asked for. The quiet always defaulting to False does seem rather odd to me as well.

The current behaviour still seems in-line with the behaviour a non-nullable charfield (if not self.null, default to '').

So, for consistency should we also make a not-null charfield fail loudly if instanciated without a value ? :P

[anb]

My argument for that is on the ticket. "It's ok for CharFields with
blank=True to default to the empty string, because that's semantically
the lack of a value for the field. However, True and False are equals;
False is not the lack of a value."

[Tobias McNulty]

Really?  Django makes the case[1] that "" means "no data" for char and text fields, as does None for integers, dates, and booleans.  As far as I can tell, the behavior of all fields, except for BooleanField, is to default to the empty value supported by that field.  Personally I see nothing wrong with that (though I suppose a case could be made against the "" default for CharFields, if someone wanted to).  On the other hand, False is in no way an "empty value."  The flip side of the question is, given the current behavior of BooleanFields, should we also make all not-null IntegerFields default to 0?

Tobias

[1] http://docs.djangoproject.com/en/dev/ref/models/fields/#null
--
Tobias McNulty, Managing Partner

Caktus Consulting Group, LLC
http://www.caktusgroup.com

[Tobias McNulty]

Whoops, looks like I should have refreshed this thread before hitting send.  :-) 

Tobias

[Richard Laager]

On Thu, 2011-02-03 at 13:46 -0500, Tobias McNulty wrote:
> the behavior of all fields, except for BooleanField, is to default to
> the empty value supported by that field.

...

> On the other hand, False is in no way an "empty value."

From the point of view of the model layer, your point makes sense.
However, I think the UI is the challenge. This would force every
BooleanField to have a drop-down (like NullBooleanField) instead of a
checkbox, at least on the /add/ forms.

Richard

[Karen Tracey]

No, that problem is already handled by the CheckboxInput widget (http://docs.djangoproject.com/en/dev/topics/db/models/#field-name-hiding-is-not-permitted), which forces "no value" to False. We're not relying on the model-level False default to get check boxes to work properly.

Karen

--
http://tracey.org/kmt/