Opened 10 years ago

Closed 4 years ago

#3997 closed Bug (fixed)

Missing default value causes exception on save

Reported by: Henrik Vendelbo <info@…> Owned by: Philippe Raoult
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: doc
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Using MySQL I have noticed a common cause for save to fail. Especially BooleanField's with no default value. If there is no default value, MySQL will return a Warning for data truncation, which will cause the save to fail. I believe that I tried to catch MySQLdb.Warning and pass on it with no improvement.

1) A warning should not cause the transaction to fail
2) If a default value is effectively required, validate should check for it

Attachments (2)

3997.diff (571 bytes) - added by Philippe Raoult 9 years ago.
note in the model api doc about that …
3997_2.diff (656 bytes) - added by Philippe Raoult 9 years ago.

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by Philippe Raoult

Attachment: 3997.diff added

note in the model api doc about that ...

comment:1 Changed 9 years ago by Philippe Raoult

Has patch: set
Keywords: doc added
Owner: changed from nobody to Philippe Raoult
Status: newassigned
Triage Stage: UnreviewedReady for checkin

The patch only adds a note to the doc. An empty Boolean fields will cause validate() to fail and save() to raise as expected.

comment:2 Changed 9 years ago by Malcolm Tredinnick

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

A couple of things about this patch:

  1. validate() doesn't work totally at the moment, so documenting it's existence would be premature. That is work-in-progress.
  2. We should try to work out 'which' types of error save() might raise, if possible, so that code can catch them if they want to. IntegrityError from the db layer is one.

I'm not totally onboard with MySQL's idea that storing inconsistent data is only a warning and not an error. I'm not sure we're behaving incorrectly there, at least in the sense that trying to save invalid data is bad.

Changed 9 years ago by Philippe Raoult

Attachment: 3997_2.diff added

comment:3 Changed 9 years ago by Philippe Raoult

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin
  1. removed the reference to validate()
  2. I added the exceptions I would find, and mentionned that it'd DB dependent.

Until we stop relying on the DB to do the checking for us there's nothing more we can do here.

comment:4 Changed 8 years ago by Marc Fargas

Triage Stage: Ready for checkinAccepted

Saying "database independant" is a bit ugly, we should catch the different exceptions and raise one common for every backend unless we want people to either catch only the one they care about (the backend they're using) or have to deal with all of them.

Anyway I'd go for setting up "traditional mode" in MySQL by default... here, note the joke in "Make MySQL behave like a “traditional” SQL database system. A simple description of this mode is “give an error instead of a warning” when inserting an incorrect value into a column."

comment:5 Changed 7 years ago by Adam Nelson

Patch needs improvement: set

Seems like there's still dispute on the best way to solve this.

comment:6 Changed 6 years ago by Łukasz Rekucki

Severity: Normal
Type: Bug

comment:7 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:8 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:9 Changed 4 years ago by Tim Graham

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top