Opened 4 years ago

Closed 2 years ago

#15124 closed Bug (fixed)

BooleanField should not use False as default (unless provided)

Reported by: andrewbadr Owned by: aaugustin
Component: Database layer (models, ORM) Version: 1.4-alpha-1
Severity: Normal Keywords:
Cc: andrewbadr.etc@…, lrekucki, 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 andrewbadr 4 years ago.
Fix + test + fix for unrelated test
15124.v2.patch (3.1 KB) - added by neaf 3 years ago.
Add documentation and release notes mention.
15124ticket.diff (51.2 KB) - added by ethlinn 3 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 3 years ago.
change marked as backwards incompatibile in release notes
15124ticket.v3.diff (3.6 KB) - added by chomik 3 years ago.
corrects 15124.v2 FlatPage BooleanFields to have default value
15124.updated.patch (5.3 KB) - added by bberes 3 years ago.
Combined and updated prev. patches to apply cleanly. Added doc entry to model fields
15124.django15.diff (4.4 KB) - added by oinopion 3 years ago.
Updated patch to current effort (1.5)

Download all attachments as: .zip

Change History (24)

comment:1 Changed 4 years ago by andrewbadr

  • Cc andrewbadr.etc@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by russellm

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Triage Stage changed from Unreviewed to Design decision needed

Discussion on django-dev

Changed 4 years ago by andrewbadr

Fix + test + fix for unrelated test

comment:3 Changed 4 years ago by andrewbadr

  • Has patch set
  • Version changed from 1.2 to 1.3-beta

comment:4 Changed 4 years ago by Alex

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

comment:5 Changed 4 years ago by andrewbadr

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 4 years ago by jaddison

  • Severity set to Normal
  • Type set to Bug

comment:7 Changed 3 years ago by aaugustin

  • Easy pickings unset
  • Triage Stage changed from Design decision needed to Accepted
  • UI/UX unset

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

Changed 3 years ago by neaf

Add documentation and release notes mention.

comment:8 Changed 3 years ago by ethlinn

  • Cc asendecka@… added
  • Needs documentation set
  • Triage Stage changed from Accepted to Ready for checkin
  • Version changed from 1.3-beta to 1.4-alpha-1

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

Changed 3 years ago by ethlinn

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

comment:9 Changed 3 years ago by ethlinn

  • Cc asendecka@… removed
  • Triage Stage changed from Ready for checkin to Accepted

Changed 3 years ago by chomik

change marked as backwards incompatibile in release notes

comment:10 Changed 3 years ago by lrekucki

  • Cc lrekucki added

comment:11 Changed 3 years ago by chomik

  • Cc to.chomik@… added

Changed 3 years ago by chomik

corrects 15124.v2 FlatPage BooleanFields to have default value

comment:12 Changed 3 years ago by lpiatek

  • Cc lpiatek added

Changed 3 years ago by bberes

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

comment:13 Changed 3 years ago by bberes

  • Cc botondus@… added
  • Needs documentation unset

comment:14 Changed 3 years ago by carljm

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 3 years ago by oinopion

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

Changed 3 years ago by oinopion

Updated patch to current effort (1.5)

comment:16 Changed 3 years ago by aaugustin

  • Owner changed from nobody to aaugustin

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

  • Resolution set to fixed
  • Status changed from new to closed

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