django sqlite autoincrement bug

[alexander lind]

I am using the svn trunk version of Django.

I was just starting a new django project using sqlite for the db
backend. Excerpt from models.py:

class User(models.Model):
user_id = models.AutoField(primary_key=True)

This produces a table in sqlite that will NOT take NULL for a value
when inserting records. You get an error back. Reading sqlites manual,
this is _supposed_ to work, but doesn't seem to. However and
furthermore, you don't really get autoincrement behavior from sqlite
unless you add in the SQL keyword "AUTOINCREMENT" when creating the
table.

Django does not do this currently, so I hacked in an option in
db.models, so I can now do:

class User(models.Model):
user_id = models.AutoField(primary_key=True, autoincrement=True)

Then I get a true autoincrement user_id field in the sqlite db.

Can the list please advise me if I am missing anything, or if I should
submit a patch in trac?

Thanks
Alec

[Malcolm Tredinnick]

On Fri, 2009-01-30 at 22:37 -0500, alexander lind wrote:
> I am using the svn trunk version of Django.
>
> I was just starting a new django project using sqlite for the db
> backend. Excerpt from models.py:
>
> class User(models.Model):
> user_id = models.AutoField(primary_key=True)
>
> This produces a table in sqlite that will NOT take NULL for a value
> when inserting records. You get an error back.

That's correct behaviour. A primary key column must be unique and not
null. By definition. No bug there.

> Reading sqlites manual,
> this is _supposed_ to work, but doesn't seem to. However and
> furthermore, you don't really get autoincrement behavior from sqlite
> unless you add in the SQL keyword "AUTOINCREMENT" when creating the
> table.
>
> Django does not do this currently, so I hacked in an option in
> db.models, so I can now do:

That's not the right solution. You're making the symptom go away, not
fixing the problem itself.

Your observation is correct: the SQLite backend doesn't add
AUTOINCREMENT. The fix is to make it always add AUTOINCREMENT. An
AutoField is an auto-increment field: it's not optional.

Shows how infrequently AutoField's are really used in practice. They're
generally just not that useful to specify.

Anyway, if you you'd like to fix your patch to always do this for the
SQLite backend, that would be great (it looks like a one-line patch to
django/db/backends/sqlite/creation.py).

Regards,
Malcolm

[alexander lind]

class User(models.Model):

user_id = models.AutoField(primary_key=True)

This produces a table in sqlite that will NOT take NULL for a value  

when inserting records. You get an error back.

That's correct behaviour. A primary key column must be unique and not
null. By definition. No bug there.

Right. I stated it because if you do an insert and just leave the autoincrementing field out of the field-list, sqlite will return the "sorry, null is not an acceptable value for this field". I was a bit unclear.

That's not the right solution. You're making the symptom go away, not
fixing the problem itself.

Your observation is correct: the SQLite backend doesn't add
AUTOINCREMENT. The fix is to make it always add AUTOINCREMENT. An
AutoField is an auto-increment field: it's not optional.

You're right.

Shows how infrequently AutoField's are really used in practice. They're
generally just not that useful to specify.

What else do people use for specifying autoinc fields?

Anyway, if you you'd like to fix your patch to always do this for the
SQLite backend, that would be great (it looks like a one-line patch to
django/db/backends/sqlite/creation.py).

Ok, will do!

Thanks

Alec

[alexander lind]

>
>> Reading sqlites manual,
>> this is _supposed_ to work, but doesn't seem to. However and
>> furthermore, you don't really get autoincrement behavior from sqlite
>> unless you add in the SQL keyword "AUTOINCREMENT" when creating the
>> table.
>>
>> Django does not do this currently, so I hacked in an option in
>> db.models, so I can now do:
>
> That's not the right solution. You're making the symptom go away, not
> fixing the problem itself.
>
> Your observation is correct: the SQLite backend doesn't add
> AUTOINCREMENT. The fix is to make it always add AUTOINCREMENT. An
> AutoField is an auto-increment field: it's not optional.
>
> Shows how infrequently AutoField's are really used in practice.
> They're
> generally just not that useful to specify.
>
> Anyway, if you you'd like to fix your patch to always do this for the
> SQLite backend, that would be great (it looks like a one-line patch to
> django/db/backends/sqlite/creation.py).

Malcolm, in fact the fix is not this easy unfortunately. I assume you
mean for me to just add "AUTOINCREMENT" in sqlite/creation.py like so:

'AutoField': 'integer', -> 'AutoField': 'integer autoincrement',

That does not do the trick however. The resulting sql becomes:
...
"user_id" integer AUTOINCREMENT NOT NULL PRIMARY KEY,
...

Which is not ok, sqlite requires it to be:
...
"user_id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
...

I don't see a simple way to make this happen. Doesn't seem like any
other backend DB requires similar behavior, so there is no support for
suffixing the sql table creation line like that.
Do you have any ideas for what the best solution would be?

Thanks
Alec

[alexander lind]

>> Anyway, if you you'd like to fix your patch to always do this for the
>> SQLite backend, that would be great (it looks like a one-line patch
>> to
>> django/db/backends/sqlite/creation.py).
>

> I don't see a simple way to make this happen. Doesn't seem like any
> other backend DB requires similar behavior, so there is no support for
> suffixing the sql table creation line like that.
> Do you have any ideas for what the best solution would be?
>

Found a way, made a patch:

http://code.djangoproject.com/ticket/10164

Alec

[Malcolm Tredinnick]

On Sat, 2009-01-31 at 16:56 -0500, alexander lind wrote:
>
> > >
> > > class User(models.Model):
> > > user_id = models.AutoField(primary_key=True)
> > >
> > > This produces a table in sqlite that will NOT take NULL for a
> > > value
> > > when inserting records. You get an error back.
> >
> > That's correct behaviour. A primary key column must be unique and
> > not
> > null. By definition. No bug there.
>
>
> Right. I stated it because if you do an insert and just leave the
> autoincrementing field out of the field-list, sqlite will return the
> "sorry, null is not an acceptable value for this field". I was a bit
> unclear.
>
> >
> > That's not the right solution. You're making the symptom go away,
> > not
> > fixing the problem itself.
> >
> > Your observation is correct: the SQLite backend doesn't add
> > AUTOINCREMENT. The fix is to make it always add AUTOINCREMENT. An
> > AutoField is an auto-increment field: it's not optional.
>
>
> You're right.
>
> >
> >
> > Shows how infrequently AutoField's are really used in practice.
> > They're
> > generally just not that useful to specify.
>
>
> What else do people use for specifying autoinc fields?

Auto-increment fields generally aren't that useful in practice, outside
of primary keys (the reasonsing being that, since they can act as
primary keys, you might as well make it the table's primary key if
you're using one. A non-primary key auto-inc field is usually a sign of
an unnecessarily denormalised data model). Since Django automatically
creates an auto-increment primary key field, the majority of the time
the manual specification isn't needed.

Regards,
Malcolm

>

[alexander lind]

>>> Shows how infrequently AutoField's are really used in practice.
>>> They're
>>> generally just not that useful to specify.
>>
>>
>> What else do people use for specifying autoinc fields?
>
> Auto-increment fields generally aren't that useful in practice,
> outside
> of primary keys (the reasonsing being that, since they can act as
> primary keys, you might as well make it the table's primary key if
> you're using one. A non-primary key auto-inc field is usually a sign
> of
> an unnecessarily denormalised data model). Since Django automatically
> creates an auto-increment primary key field, the majority of the time
> the manual specification isn't needed.

I always make my auto-inc fields primary as well, so no argument there.
I tried using the AutoField when I noticed django didn't create the
auto-incrementing fields correctly by itself in sqlite, but that
didn't work either until I patched it.

Alec

[Malcolm Tredinnick]

Then something else is going on and maybe SQLite doesn't need the
AUTOINCREMENT marker for some reason. Because automatic primary key
fields work fine with SQLite. If that ever broke, we'd hear about it
very quickly.

Regards,
Malcolm

[alexander lind]

On Feb 2, 2009, at 10:05 PM, Malcolm Tredinnick wrote:

I always make my auto-inc fields primary as well, so no argument there.

I tried using the AutoField when I noticed django didn't create the  

auto-incrementing fields correctly by itself in sqlite, but that  

didn't work either until I patched it.

Then something else is going on and maybe SQLite doesn't need the
AUTOINCREMENT marker for some reason. Because automatic primary key
fields work fine with SQLite. If that ever broke, we'd hear about it
very quickly.

According to sqlite themselves, you can create a primary key without using the AUTOINCREMENT marker, and this key will _almost_ act as an autoinc field, but there is one important difference, and that is that without the use of AUTOINCREMENT, sqlite does not guarantee that a new ID is created for each insert. If you for example deleted the latest row in the table, sqlite would re-use the ID of the deleted row for the next insert. This is because without AUTOINCREMENT, the primary key becomes an alias for sqlite:s internal ROWID, that works this way.

From their manual:

The behavior implemented by the AUTOINCREMENT keyword is subtly different from the default behavior. With AUTOINCREMENT, rows with automatically selected ROWIDs are guaranteed to have ROWIDs that have never been used before by the same table in the same database. And the automatically generated ROWIDs are guaranteed to be monotonically increasing. These are important properties in certain applications. But if your application does not need these properties, you should probably stay with the default behavior since the use of AUTOINCREMENT requires additional work to be done as each row is inserted and thus causes INSERTs to run a little slower. 

http://www.sqlite.org/autoinc.html

I think django should use the AUTOINCREMENT marker on the AutoField myself, makes sense to me.

Still, sqlite does not behave as documented above for me. If I try to use just the ROWID as primary key, I end up not being able to insert any new rows because it will just tell me that the primary key may not be null (when leaving the field out of the insert query, like you would an auto-inc field). This is not djangos fault of course, just an oddity that I really don't understand.

Alec

[Malcolm Tredinnick]

On Mon, 2009-02-02 at 23:16 -0500, alexander lind wrote:
>
> On Feb 2, 2009, at 10:05 PM, Malcolm Tredinnick wrote:
> > > I always make my auto-inc fields primary as well, so no argument
> > > there.
> > > I tried using the AutoField when I noticed django didn't create
> > > the
> > > auto-incrementing fields correctly by itself in sqlite, but that
> > > didn't work either until I patched it.
> >
> > Then something else is going on and maybe SQLite doesn't need the
> > AUTOINCREMENT marker for some reason. Because automatic primary key
> > fields work fine with SQLite. If that ever broke, we'd hear about it
> > very quickly.
>
>
> According to sqlite themselves, you can create a primary key without
> using the AUTOINCREMENT marker, and this key will _almost_ act as an
> autoinc field, but there is one important difference, and that is that
> without the use of AUTOINCREMENT, sqlite does not guarantee that a new
> ID is created for each insert. If you for example deleted the latest
> row in the table, sqlite would re-use the ID of the deleted row for
> the next insert. This is because without AUTOINCREMENT, the primary
> key becomes an alias for sqlite:s internal ROWID, that works this way.

Okay, that sounds like the behaviour we need. So I'm comfortable that
there isn't a major bug in Django now.

[...]

> I think django should use the AUTOINCREMENT marker on the AutoField
> myself, makes sense to me.

It's possible, but not a requirement. Since the documentation recommends
what we're doing now, our current choice seems like a good idea, too.

In the absence of any concrete bug and a real use-case that requires a
change, I'd be unenthusiastic about changing current behaviour.

> Still, sqlite does not behave as documented above for me. If I try to
> use just the ROWID as primary key, I end up not being able to insert
> any new rows because it will just tell me that the primary key may not
> be null (when leaving the field out of the insert query, like you
> would an auto-inc field).

So you're trying to do something a bit weird then, since at least a few
thousand people use SQLite in Django applications all the time without
problems. I still don't really understand what it is you're trying to
do, but I've decided I'm not particularly worried by it.

Auto-generated primary keys in SQLite work fine. You can happily create
and save models with them. If you want to manually declare the primary
key field, go for it. You can even use AutoField and it will work fine
with Django. If you want a field with different behaviour from
AutoField, that's also fine, since it's easy to create custom fields for
custom database types -- we've put a lot of effort into making that
possible.

Regards,
Malcolm