Opened 13 years ago

Closed 11 years ago

#15124 closed Bug (fixed)

BooleanField should not use False as default (unless provided)

Reported by: Andrew Badr Owned by: Aymeric Augustin
Component: Database layer (models, ORM) Version: 1.4-alpha-1
Severity: Normal Keywords:
Cc: andrewbadr.etc@…, Łukasz Rekucki, to.chomik@…, lpiatek, botondus@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Steps to repro:
1) Make a Model with a BooleanField with no default
2) Instantiate that model with no initial value
3) The value of the BooleanField on that instance is False

Expected behavior:
3) The value of the BooleanField on that instance is None

Guessing a value for the field is wrong. Attempting to save a model with a non-null field without providing a value for that field should be an error. (Postgres of course throws an error if you try to insert a row with no value for a required bool field.) 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.

Note the existence of the opposite ticket, #2855, which was wontfix'd then apparently silently "fixed" at some point.

Attachments (7)

t15124_r1.2.4_v1.diff (2.1 KB ) - added by Andrew Badr 13 years ago.
Fix + test + fix for unrelated test
15124.v2.patch (3.1 KB ) - added by neaf 12 years ago.
Add documentation and release notes mention.
15124ticket.diff (51.2 KB ) - added by Aleksandra Sendecka 12 years ago.
Fix with tests and docs with realease notes for 1.4-beta-1 instead of 1.4
15124ticket.v2.diff (51.2 KB ) - added by chomik 12 years ago.
change marked as backwards incompatibile in release notes
15124ticket.v3.diff (3.6 KB ) - added by chomik 12 years ago.
corrects 15124.v2 FlatPage BooleanFields to have default value
15124.updated.patch (5.3 KB ) - added by Béres Botond 12 years ago.
Combined and updated prev. patches to apply cleanly. Added doc entry to model fields
15124.django15.diff (4.4 KB ) - added by Tomek Paczkowski 12 years ago.
Updated patch to current effort (1.5)

Download all attachments as: .zip

Change History (24)

comment:1 by Andrew Badr, 13 years ago

Cc: andrewbadr.etc@… added

comment:2 by Russell Keith-Magee, 13 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedDesign decision needed

Discussion on django-dev

by Andrew Badr, 13 years ago

Attachment: t15124_r1.2.4_v1.diff added

Fix + test + fix for unrelated test

comment:3 by Andrew Badr, 13 years ago

Has patch: set
Version: 1.21.3-beta

comment:4 by Alex Gaynor, 13 years ago

So apparently I hadn't noticed this before, but this will almost certainly break code I have, likely others as well.

comment:5 by Andrew Badr, 13 years ago

Likely, but the current behavior is (1) wrong, (2) undocumented, (3) arguably untested, and (4) has already changed in the past. The bug should be fixed for 1.3 but (as Russ says) not backported.

comment:6 by James Addison, 13 years ago

Severity: Normal
Type: Bug

comment:7 by Aymeric Augustin, 12 years ago

Easy pickings: unset
Triage Stage: Design decision neededAccepted
UI/UX: unset

I just read the thread and it seems to me that the majority is in favor of fixing this bug.

by neaf, 12 years ago

Attachment: 15124.v2.patch added

Add documentation and release notes mention.

comment:8 by Aleksandra Sendecka, 12 years ago

Cc: asendecka@… added
Needs documentation: set
Triage Stage: AcceptedReady for checkin
Version: 1.3-beta1.4-alpha-1

Works ok. Tests and docs are there, so RFC.

by Aleksandra Sendecka, 12 years ago

Attachment: 15124ticket.diff added

Fix with tests and docs with realease notes for 1.4-beta-1 instead of 1.4

comment:9 by Aleksandra Sendecka, 12 years ago

Cc: asendecka@… removed
Triage Stage: Ready for checkinAccepted

by chomik, 12 years ago

Attachment: 15124ticket.v2.diff added

change marked as backwards incompatibile in release notes

comment:10 by Łukasz Rekucki, 12 years ago

Cc: Łukasz Rekucki added

comment:11 by chomik, 12 years ago

Cc: to.chomik@… added

by chomik, 12 years ago

Attachment: 15124ticket.v3.diff added

corrects 15124.v2 FlatPage BooleanFields to have default value

comment:12 by lpiatek, 12 years ago

Cc: lpiatek added

by Béres Botond, 12 years ago

Attachment: 15124.updated.patch added

Combined and updated prev. patches to apply cleanly. Added doc entry to model fields

comment:13 by Béres Botond, 12 years ago

Cc: botondus@… added
Needs documentation: unset

comment:14 by Carl Meyer, 12 years ago

My take here is that this is a legitimate bugfix and should go in, but there's enough potential for breakage of existing code that I'm hesitant to drop it in after the beta, I'd rather target it for 1.5 at this point.

comment:15 by Tomek Paczkowski, 12 years ago

We're after 1.4. This should go in now, I guess.

by Tomek Paczkowski, 12 years ago

Attachment: 15124.django15.diff added

Updated patch to current effort (1.5)

comment:16 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin

comment:17 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: newclosed

In e16c48e001ccd06830bb0bfd1d20e22ec30fce59:

Fixed #15124 -- Changed the default for BooleanField.

Thanks to the many contributors who updated and improved the patch over
the life of this ticket.

Note: See TracTickets for help on using tickets.
Back to Top