Opened 11 years ago

Closed 6 years ago

Last modified 6 years ago

#10164 closed Bug (fixed)

sqlite3 backend: AutoField values aren't monotonically increasing

Reported by: malte Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: sqlite autoincrement
Cc: ivazquez, Matthew Flanagan, mike@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


If you try to use the 'AutoField' with sqlite, django will create a primary key table. This is not an autoincrementing table, as per sqlites manual.
In order to create a true autoinc field in sqlite (one that guarantees all rows unique id:s in the autoinc field) you must suffix the field creation line with 'AUTOINCREMENT', and this must come last in the field syntax.

The attached path implements a data_types_suffix dictionary, that can be optionally added to each backend in its file, to accomplish adding the 'AUTOINCREMENT' directive. Sqlite and its special way to create autoinc fields is the only case I know of that needs this functionality.

Attachments (2)

sqlite_autoincrement_fix.diff (2.1 KB) - added by malte 11 years ago.
fixes the problem described in the ticket
Django-1.3-sqlite.patch (2.2 KB) - added by Jonttu 9 years ago.
Patch modified for 1.3

Download all attachments as: .zip

Change History (40)

Changed 11 years ago by malte

fixes the problem described in the ticket

comment:1 Changed 11 years ago by ivazquez

Cc: ivazquez added

comment:2 Changed 11 years ago by Jacob

Triage Stage: UnreviewedDesign decision needed

I'm positive that AutoField isn't "broken" for sqlite: if it was, we'd be seeing a million test failures on sqlite since nearly every test case in the tree uses an AutoField. Since currently all tests pass on sqlite, I have to assume there's something more going on here.

So, can you clarify what you mean by "broken"? What's the problem you're seeing?

comment:3 Changed 11 years ago by Malcolm Tredinnick

Presumably this is in reference to this django-users thread. There was a bit of confusion as to the problem being reported there initially, but I think I'm happy saying in the end that there isn't really a bug here, except for a minor documentation/expectation issue.

We say (in the docs) that AutoField is an always-incrementing field, but it could go back and reuse some old numbers, the way we've implemented it for !SQLite. I'd probably vote for just fixing the docs here, simply because I prefer maintaining consistency in the SQL we produce, unless there's a functionality bug.

comment:4 Changed 10 years ago by fwenzel

This should be included. The patch looks like a good solution for it.

In spots where database IDs are exposed publicly, for example as paths: /item/1, deleting the latest item and adding a new one will result in the same ID (and thus, path) being reassigned in SQLite only, and only because Django is not using the auto_increment feature SQLite provides.

For the sake of consistency between DB backends, I suggest accepting the patch.

If needed, I could add a test ensuring the PKs Django generates increase monotonously.

comment:5 Changed 10 years ago by fwenzel

And by monotonously I mean monotonically, of course :)

comment:6 Changed 10 years ago by Ramiro Morales

#13852 also reported this.

comment:7 Changed 9 years ago by Matthew Flanagan


This issue becomes a problem when using GenericRelations and sqlite3.

from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class TaggedItem(models.Model):
    tag = models.SlugField()
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey('content_type', 'object_id')

    def __unicode__(self):
        return self.tag

class Item(models.Model):
    name = models.CharField()

    def __unicode__(self):

Now if I create an Item and assign a tag to it, delete the Item, then create a new Item the tags remain and now point to the new Item. e.g.

>>> from myapp.models import TaggedItem, Item
>>> from django.contrib.contenttypes.models import ContentType
>>> i1 = Item(name='one')
>>> t = TaggedItem(content_object=i1, tag='foo')
>>> item_type = ContentType.objects.get_for_model(i1)
>>> TaggedItem.objects.filter(, object_id=1)
[<TaggedItem: foo>]
>>> i1.delete()
>>> i2 = Item(name='two')
>>> TaggedItem.objects.filter(, object_id=1)
[<TaggedItem: foo>]

Changing sqlite3 backend to autoincrement would make it behave like postgres. I don't have mysql or oracle available to test their behaviour.

comment:8 Changed 9 years ago by Matthew Flanagan

Cc: Matthew Flanagan added

comment:9 Changed 9 years ago by whardier

Is there a status update for this? This appears to not be the case with Django 1.2.3 at the moment.

Simply altering the table after the fact appears to be a fine solution for me.

comment:10 Changed 9 years ago by philipn

mattimustang, mysql and oracle behave the same way as postgresql.

This bug has hit me a few times. If all tests pass, is there any reason this shouldn't be committed?

comment:11 Changed 9 years ago by Luke Plant

mattimustang, the problem with your example is that as soon as you delete the original object, you ought to delete the TaggedItem pointing to it, so that is an example of user error.

More compelling use cases are:

  • URLs that expose PKs, and ought to become 404s when the object is deleted, as fwenzel said.
  • Log/audit records that reference the PK, which become misleading when the object is deleted and the PK re-used, and then horribly confused. This is the most compelling example IMO, as your audit records are all being silently corrupted (in effect), and you won't find out until you try to re-construct some data from your audit tables.

comment:12 Changed 9 years ago by Matthew Flanagan


Your second use case is exactly the problem I have. The example code I wrote above was just to illustrate the issue in as simple a way as possible.

comment:13 Changed 9 years ago by Jonttu

I have a model caching system that relies on unique-not-reused pks and would like to see this patch as part of future django releases

comment:14 Changed 9 years ago by lfagundes

Version: 1.01.2

+ 1

Both Postgres and MySQL do not reuse primary keys, but in django's current implementation sqlite does. My tests could run in 2 minutes in sqlite if this bug was fixed (and it looks like the big discussion is considering this a bug), but instead I have to wait for 10 minutes for them to run in MySQL.

Please, include this patch in future releases.

comment:15 Changed 9 years ago by johnsmith

milestone: 1.4

2 years seems like a long time for this nobrainer to be outstanding.

Auto increment that doesn't reuse primary keys is the default behavior of the overwhelming majority of production databases (eg MySQL & Postgres), why wouldn't we want that behavior in SQLite (the 'normal' dev environment) as well? Is anyone going to complain if SQLite followed this behavior as well (I think not)?

comment:16 in reply to:  15 Changed 9 years ago by Luke Plant

Patch needs improvement: set
Triage Stage: Design decision neededAccepted

Replying to johnsmith:

2 years seems like a long time for this nobrainer to be outstanding.

We need compelling reasons to change something like this, because it is entirely possible that someone is depending on the existing behaviour in production i.e. people using SQLite for production. So just getting consistency for the sake of people using SQLite for testing only isn't in itself the most important consideration.

And we certainly didn't have a compelling reason for the change 2 years ago. I think we do now though, and no-one has responded otherwise, so I'm going to accept. If Jacob or Malcolm are still unsure, we probably need to have a discussion on django-devs.

The patch needs some work to be updated for trunk. Also, if there is a nice way to do it other than adding an ad-hoc feature addition to the DB backend that is needed by just this exception, that would be good.

We also need tests, either by testing the SQL generated, or by a functional test that ensures that IDs are not re-used.

comment:17 Changed 9 years ago by Chris Beaven

Severity: Normal
Summary: AutoField is broken for sqlite backend [patch included]AutoField is broken for sqlite backend
Type: Bug

comment:18 Changed 9 years ago by Jonttu

This problem exists also in South

Changed 9 years ago by Jonttu

Attachment: Django-1.3-sqlite.patch added

Patch modified for 1.3

comment:19 Changed 9 years ago by Luke Plant

Easy pickings: unset
Needs documentation: set
Needs tests: set

This still needs tests (sorry for not setting the flag before).

I'm still bothered by the creation of a new method on Field to cope with one peculiarity of a single backend, i.e. the need for the autoincrement keyword and the fact that it only allows this keyword at the end, not at the beginning. But I can't see a nicer way of doing it. As to the current patch:

In Field.db_type_suffix, I don't see any need for using the string interpolation using the dictionary. Since we have no use case for that, it would be better to remove it.

We should put db_types_suffix as an empty dictionary on BaseDatabaseCreation, and adjust the error handling in Field.db_type_suffix accordingly i.e. so it only catches KeyError.

We also need some docs - a note in the release notes, for the following reasons:

1) It could potentially cause breakage.
2) The change will require a manual migration for anyone using SQLite in production, if they care about the change. Unfortunately there is no way to do an 'alter column' in SQLite, so this migration is non-trivial.

Many thanks.

comment:20 Changed 8 years ago by Jacob

milestone: 1.4

Milestone 1.4 deleted

comment:21 Changed 8 years ago by Ramiro Morales

UI/UX: unset

#16992 reports a similar MySQL/InnoDB particularity: The DB engine reuses AUTO_INCREMENT when/after it is restarted.

Last edited 8 years ago by Ramiro Morales (previous) (diff)

comment:22 Changed 8 years ago by Ramiro Morales

Summary: AutoField is broken for sqlite backendsqlite3 backend: AutoField values aren't monotonically increasing

comment:23 Changed 7 years ago by tt@…

I was bitten by this today, and to me the AUTOINCREMENT behavior seems more reasonable than the "integer primary key" behavior.
However, I see that the potential for breakage is very much in place, so changing the default might not be the greatest of ideas.

However, if we could grow an option on the AutoField to get AUTOINCREMENT behavior, I would be very +1 on that.

comment:24 Changed 7 years ago by ad-65

I spent the afternoon tracking down a problem where my audit logs were all messed up. It was caused by this issue and using generic relations. If a row is deleted in SQLite without the 'AUTOINCREMENT' keyword the id is re-used, screwing up my audit trail in the process. I think there are probably quite a few people using SQLite for development and MySQL/PostgreSQL for production. It would be nice if the behaviour was the same.

Having said that I understand about not wanting to change core behaviour after the fact. Would it be possible to add some kind of configuration option to get the 'AUTOINCREMENT' keyword in the table creation SQL as suggested in ?

comment:25 Changed 7 years ago by anonymous

Is there any workaround to have not re-used ids for Django 1.4.3 model with SQLite?

comment:26 Changed 7 years ago by Luke Plant

To recent commenters:

This bug is accepted - the current behaviour is regarded as a bug, and we will accept good patches fixing this problem. The limitation is someone willing to write a good patch. You can see my comments on the previous patch above.

However, I should say that you really should consider a better database for production. Even for development, differences between development and production can really bite. Because of this, all experienced Django developers only use SQLite as a toy, strictly for quick throw-away databases and experiments, and so none of us are likely to write a patch for this.

But patches are welcome!

Last edited 7 years ago by Luke Plant (previous) (diff)

comment:27 Changed 7 years ago by pacemkr@…

I was also bitten by this today. SQLite is very convenient for early stages of a new project.

contentypes + this issue results in some very weird behavior when you delete something.

comment:28 Changed 7 years ago by Mike Fogel

Cc: mike@… added

comment:29 Changed 6 years ago by Chris Wilson

Submitted PR with test and documentation, and changes requested by @lukeplant.

comment:30 Changed 6 years ago by Anssi Kääriäinen

I wonder about migrations support for this. We have that now in core...

Otherwise the approach looks good to me. The backwards incompatiblity of this seems minor.

comment:31 Changed 6 years ago by Chris Wilson

@akaariai how would we do migrations using what's in core? Isn't it up to developers to freeze their models before and after the upgrade to ensure that a migration takes place, if they want it?

Otherwise we don't automatically muck about with their tables, which seems like a good thing to me.

comment:32 Changed 6 years ago by Anssi Kääriäinen

Truthfully, I don't know. I have to ask and see if there is anything that can be done here. It would be nice to have a upgrade script one can run if that is wanted, but which isn't ran automatically. I don't know if that is possible with current migrations framework...

comment:33 Changed 6 years ago by Tim Graham

Needs documentation: unset
Needs tests: unset

I don't think the migrations framework is really designed to handle this type of thing, particularly since it's SQLite specific. See the migration docs for how migrations work for SQLite. I don't feel we need to provide much help with a respect to a migration here, particularly if a lack of solution is going to further delay fixing this. I've left some minor comments on the PR.

comment:34 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In eade315da1c8372ac1dfcf1fd20ea87f454d71ac:

Fixed #10164 -- Made AutoField increase monotonically on SQLite

Thanks malte for the report.

comment:35 Changed 6 years ago by Aymeric Augustin

Resolution: fixed
Status: closednew

I'm sorry, but the release notes look wrong to me. Do you think my proposal makes sense?

comment:36 Changed 6 years ago by Tim Graham <timograham@…>

In 910a5760f6f5ffc42b4b264b08057207b8e26106:

Improved release notes for ticket #10164

Thanks Aymeric for the suggestions.

refs #10164

comment:37 Changed 6 years ago by Tim Graham

Resolution: fixed
Status: newclosed

comment:38 Changed 6 years ago by Ramiro Morales <cramm0@…>

In 6a6136141b133aff7583d51d687719961d261244:

Implemented #10164 for new schema migration code.

Made it use 'AUTOINCREMENT' suffix for PK creation. This way it doeesn't
regress when compared with the 'traditional' DB backend creation

Refs #10164.

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