Opened 18 years ago

Closed 18 years ago

Last modified 16 years ago

#2108 closed defect (fixed)

Empty models result in invalid SQL for Sqlite

Reported by: andy@… Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version:
Severity: normal Keywords: sqlite
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A model with no fields, e.g.

class EmptyModel(models.Model):

pass

will cause an OperationalError to be raised when an instance is saved:

m = EmptyModel()
m.save()

if the database is sqlite3.
In this case the SQL executed is:

INSERT INTO "testapp_emptymodel" () VALUES ()

It should instead be generating the following SQL:

INSERT INTO "testapp_emptymodel" (id) VALUES (NULL)

Change History (7)

comment:1 by Malcolm Tredinnick, 18 years ago

This is indeed a bug, but the proposed solution is not correct. A primary key in a database is unique and not NULL, so trying to set id to be NULL is not a good idea, even if it might happen to work in SQLite. We should be not saving anything in this case.

comment:2 by Malcolm Tredinnick, 18 years ago

Resolution: fixed
Status: newclosed

(In [3104]) Fixed #2108 -- do not try to save an empty model.

comment:3 by andy@…, 18 years ago

Resolution: fixed
Status: closedreopened

Inserting NULL into an automatically-incremented field is a common idiom that works in sqlite, mysql, and sql server at least.

Regardless, your solution -- to not save anything -- is wrong. There are cases where a model might have no fields but an id. The one I've got at the moment is a shopping basket, which at the moment does not refer to any other models -- but other models refer to it. If the basket silently fails to be saved, then the other model objects will also fail, all without a clear reason.

Either Django should support tables with just an id field, or it should throw a ValidationError or some such. Silently failing is not good.

comment:4 by Malcolm Tredinnick, 18 years ago

Owner: changed from Adrian Holovaty to Malcolm Tredinnick
Status: reopenednew

I understand your use case now. Sounds reasonable, although a bit of an unusual case.

Inserting NULL in that field is, as a rule, not correct -- it is invalid SQL and it's sort of a shame that MySQL and SQL server permit it. PostgreSQL rejects it. The main problem is SQLite. We should be executing insert into mytable (id) values (DEFAULT) but SQLite does not understand the standard DEFAULT value, so you must use NULL in that case for integer primary keys.

I have made a patch that now inserts the new elements each time and has a backend-specific hook to get the right value to insert (either DEFAULT or NULL, as appropriate). Will commit it in a moment and get somebody on the mailing list to test the SQL Server case.

comment:5 by Malcolm Tredinnick, 18 years ago

Status: newassigned

comment:6 by Malcolm Tredinnick, 18 years ago

Resolution: fixed
Status: assignedclosed

(In [3115]) Fixed #2108 -- allow saving of empty models, rather than just dropping them.

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