Opened 6 years ago

Closed 4 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 6 years ago.
Fix + test + fix for unrelated test
15124.v2.patch (3.1 KB) - added by neaf 5 years ago.
Add documentation and release notes mention.
15124ticket.diff (51.2 KB) - added by Aleksandra Sendecka 5 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 5 years ago.
change marked as backwards incompatibile in release notes
15124ticket.v3.diff (3.6 KB) - added by chomik 5 years ago.
corrects 15124.v2 FlatPage BooleanFields to have default value
15124.updated.patch (5.3 KB) - added by Béres Botond 5 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 5 years ago.
Updated patch to current effort (1.5)

Download all attachments as: .zip

Change History (24)

comment:1 Changed 6 years ago by Andrew Badr

Cc: andrewbadr.etc@… added

comment:2 Changed 6 years ago by Russell Keith-Magee

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

Discussion on django-dev

Changed 6 years ago by Andrew Badr

Attachment: t15124_r1.2.4_v1.diff added

Fix + test + fix for unrelated test

comment:3 Changed 6 years ago by Andrew Badr

Has patch: set
Version: 1.21.3-beta

comment:4 Changed 6 years ago by Alex Gaynor

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

comment:5 Changed 6 years ago by Andrew Badr

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 Changed 6 years ago by James Addison

Severity: Normal
Type: Bug

comment:7 Changed 5 years ago by Aymeric Augustin

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.

Changed 5 years ago by neaf

Attachment: 15124.v2.patch added

Add documentation and release notes mention.

comment:8 Changed 5 years ago by Aleksandra Sendecka

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.

Changed 5 years ago by Aleksandra Sendecka

Attachment: 15124ticket.diff added

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

comment:9 Changed 5 years ago by Aleksandra Sendecka

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

Changed 5 years ago by chomik

Attachment: 15124ticket.v2.diff added

change marked as backwards incompatibile in release notes

comment:10 Changed 5 years ago by Łukasz Rekucki

Cc: Łukasz Rekucki added

comment:11 Changed 5 years ago by chomik

Cc: to.chomik@… added

Changed 5 years ago by chomik

Attachment: 15124ticket.v3.diff added

corrects 15124.v2 FlatPage BooleanFields to have default value

comment:12 Changed 5 years ago by lpiatek

Cc: lpiatek added

Changed 5 years ago by Béres Botond

Attachment: 15124.updated.patch added

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

comment:13 Changed 5 years ago by Béres Botond

Cc: botondus@… added
Needs documentation: unset

comment:14 Changed 5 years ago by Carl Meyer

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 Changed 5 years ago by Tomek Paczkowski

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

Changed 5 years ago by Tomek Paczkowski

Attachment: 15124.django15.diff added

Updated patch to current effort (1.5)

comment:16 Changed 5 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin

comment:17 Changed 4 years ago by Aymeric Augustin <aymeric.augustin@…>

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