Allow overriding of default sequence name

[abuse@…]

I wanted to be able to use Django's admin interface to edit PostgreSQL inherited tables. The normal use would be to have a single sequence on the parent table and let the children inherit that, so that the IDs across all children are unique. However, Django assumes that the sequence name is tablename_columnname_seq which does not apply in this case.

I have created the folloiwing patch to allow one to override the default sequence name with one of your choosing by adding a seq_name= parameter when defining the field, like this:

class PeerIAX(models.Model):
    class Admin: pass
    id = models.AutoField(primary_key=True, seq_name='asterisk_peer_id_seq')
    description = models.CharField(maxlength=255)
    hostname = models.CharField(maxlength=255)
    username = models.CharField(maxlength=255)
    secret = models.CharField(maxlength=255)
    def __str__(self):
	return description

In case it's not obvious, this simple patch does not add support to Django to actually *create* the tables. I hand-created them like so:

CREATE TABLE asterisk_peer (
	id		SERIAL		NOT NULL PRIMARY KEY,
	description	VARCHAR(255)	NOT NULL
);

CREATE TABLE asterisk_peer_iax (
	hostname	VARCHAR(255)	NOT NULL,
	username	VARCHAR(255)	NOT NULL,
	secret		VARCHAR(255)	NOT NULL
) INHERITS (asterisk_peer);

The patch is below. I abandon copyright on the code and you are welcome to incorporate the patch into Django as you see fit.

diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/backends/ado_mssql/base.py ./django/db/backends/ado_mssql/base.py
--- ../Django-0.91-py2.3.egg.orig/django/db/backends/ado_mssql/base.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/backends/ado_mssql/base.py	2006-05-20 20:09:24.000000000 +0100
@@ -94,7 +94,9 @@
 dictfetchmany = util.dictfetchmany
 dictfetchall  = util.dictfetchall
 
-def get_last_insert_id(cursor, table_name, pk_name):
+def get_last_insert_id(cursor, table_name, pk_name, seq_name):
+    # FIXME: seq_name might be a useful thing to add to backends other than
+    # PostgreSQL
     cursor.execute("SELECT %s FROM %s WHERE %s = @@IDENTITY" % (pk_name, table_name, pk_name))
     return cursor.fetchone()[0]
 
diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/backends/mysql/base.py ./django/db/backends/mysql/base.py
--- ../Django-0.91-py2.3.egg.orig/django/db/backends/mysql/base.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/backends/mysql/base.py	2006-05-20 20:10:01.000000000 +0100
@@ -117,7 +117,9 @@
 dictfetchmany = util.dictfetchmany
 dictfetchall  = util.dictfetchall
 
-def get_last_insert_id(cursor, table_name, pk_name):
+def get_last_insert_id(cursor, table_name, pk_name, seq_name):
+    # FIXME: seq_name might be a useful thing to add to backends other than
+    # PostgreSQL
     return cursor.lastrowid
 
 def get_date_extract_sql(lookup_type, table_name):
diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/backends/postgresql/base.py ./django/db/backends/postgresql/base.py
--- ../Django-0.91-py2.3.egg.orig/django/db/backends/postgresql/base.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/backends/postgresql/base.py	2006-05-20 19:40:12.000000000 +0100
@@ -75,8 +75,10 @@
     "Returns all rows from a cursor as a dict"
     return cursor.dictfetchall()
 
-def get_last_insert_id(cursor, table_name, pk_name):
-    cursor.execute("SELECT CURRVAL('\"%s_%s_seq\"')" % (table_name, pk_name))
+def get_last_insert_id(cursor, table_name, pk_name, seq_name):
+    if seq_name is None:
+        seq_name = "%s_%s_seq" % (table_name, pk_name)
+    cursor.execute("SELECT CURRVAL('\"%s\"')" % seq_name)
     return cursor.fetchone()[0]
 
 def get_date_extract_sql(lookup_type, table_name):
diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/backends/sqlite3/base.py ./django/db/backends/sqlite3/base.py
--- ../Django-0.91-py2.3.egg.orig/django/db/backends/sqlite3/base.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/backends/sqlite3/base.py	2006-05-20 20:10:11.000000000 +0100
@@ -90,7 +90,9 @@
 dictfetchmany = util.dictfetchmany
 dictfetchall  = util.dictfetchall
 
-def get_last_insert_id(cursor, table_name, pk_name):
+def get_last_insert_id(cursor, table_name, pk_name, seq_name):
+    # FIXME: seq_name might be a useful thing to add to backends other than
+    # PostgreSQL
     return cursor.lastrowid
 
 def get_date_extract_sql(lookup_type, table_name):
diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/models/base.py ./django/db/models/base.py
--- ../Django-0.91-py2.3.egg.orig/django/db/models/base.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/models/base.py	2006-05-20 19:55:52.000000000 +0100
@@ -187,7 +187,7 @@
                 (backend.quote_name(self._meta.db_table), ','.join(field_names),
                 ','.join(placeholders)), db_values)
             if self._meta.has_auto_field and not pk_set:
-                setattr(self, self._meta.pk.attname, backend.get_last_insert_id(cursor, self._meta.db_table, self._meta.pk.column))
+                setattr(self, self._meta.pk.attname, backend.get_last_insert_id(cursor, self._meta.db_table, self._meta.pk.column, self._meta.pk.seq_name))
         transaction.commit_unless_managed()
 
         # Run any post-save hooks.
diff -rduP ../Django-0.91-py2.3.egg.orig/django/db/models/fields/__init__.py ./django/db/models/fields/__init__.py
--- ../Django-0.91-py2.3.egg.orig/django/db/models/fields/__init__.py	2006-05-15 13:52:49.000000000 +0100
+++ ./django/db/models/fields/__init__.py	2006-05-20 19:53:28.000000000 +0100
@@ -68,7 +68,7 @@
         core=False, rel=None, default=NOT_PROVIDED, editable=True,
         prepopulate_from=None, unique_for_date=None, unique_for_month=None,
         unique_for_year=None, validator_list=None, choices=None, radio_admin=None,
-        help_text='', db_column=None):
+        help_text='', db_column=None,seq_name=None):
         self.name = name
         self.verbose_name = verbose_name
         self.primary_key = primary_key
@@ -84,6 +84,7 @@
         self.radio_admin = radio_admin
         self.help_text = help_text
         self.db_column = db_column
+        self.seq_name = seq_name
 
         # Set db_index to True if the field has a relationship and doesn't explicitly set db_index.
         self.db_index = db_index

[nirvdrum]

Another use case is integrating with legacy databases. I just found out I can't really use auto-increment IDs with django until something like this patch gets integrated because the legacy sequence names are not what django expects.

[Malcolm Tredinnick]

I'm a pretty big fan of helping out legacy database support where we can do it without too much trouble (not so sold on the initial reasons in the ticket, but if it works for that, too... great). I think the idea is not unreasonable, although the patch needs some work. A couple of things that jump out at me:

1. get_last_insert_id() should take seq_name=None as a default parameter, rather than hoping the callers will know that None is the "do nothing" default. That also makes it more backwards compatible if anybody is already calling that function from other places.
2. django/core/management.py needs to be fixed to create the sequence correctly. We don't want something that works except for the fact that you have to manually create it: that would be unusual behaviour for Django.
3. I don't like the "FIXME" comment that has "maybe" in it: sounds like putting a design query in the code. We just document the seq_name attribute as being only applicable to PostgreSQL for now (and add the others to the docs if/when they are implemented).

This should be testable and needs a docs patch as well.

[nirvdrum@…]

I'm willing to step up to the plate with regards to the docs patch. I'm not sure I have the requisite knowledge to provide an adequate patch for testing. If no one else can step up to the plate, I'll try to, but no guarantees I'll get anywhere with it.

[hanne.moa@…]

Another case of global sequence in an existing db here so I also need this (and it is a showstopper). Where exactly should this be in the documentation, since it is (currently) only needed for postgres? It might be *useful* for other dbs also, but...

[hanne.moa@…]

Custom sequence-name patch for trunk (v7156)

[hanne.moa@…]

Tests for patch (or a good start anyway)

[hanne.moa@…]

I noticed that the oracle backend currently uses get_sequence_name() in autoinc_sql() as well as changing last_insert_id(), this might in fact be a better solution as it is possible to ask postgres which sequence a field uses, which is a lot more DRY. Might be slower, though. A combined solution would be to set/cache a sequence_name at instantiation of the model then use that all other places, making adding an attribute to AutoField's init unneccessary. Things would just work and no new docs would be needed... The test I attached is still relevant though.

Get sequence-name from postgres:

<database>=# SELECT * FROM pg_get_serial_sequence('<tablename>', '<fieldname>');
 pg_get_serial_sequence
------------------------
 public.<sequence_name>
(1 row)

Unfortunately, in the case of inherited tables (see ​Table-inheritance in postgresql)you need to check the parent-table, not the child. This is worth testing for if the above sql returns NULL.

[anonymous]

I subclassed a database-backend while waiting for a solution to this ticket to wind up in trunk, as according to ​http://www.djangoproject.com/documentation/settings/#database-engine this should be possible:

In the Django development version, you can use a database backend that doesn’t 
ship with Django by setting DATABASE_ENGINE to a fully-qualified path (i.e. 
mypackage.backends.whatever). Writing a whole new database backend from
scratch is left as an exercise to the reader; see the other backends for examples.

However as of trunk v. 7167, this is not possible, failing with:

  ..
  File "/usr/lib/python2.5/site-packages/django/db/__init__.py", line 32, in <module>
    (settings.DATABASE_ENGINE, ", ".join(map(repr, available_backends)))
django.core.exceptions.ImproperlyConfigured: 'extra.backends.psycopg2_sequencesafe' isn't 
an available database backend. Available options are: 'ado_mssql', 'dummy', 'mysql', 
'mysql_old', 'oracle', 'postgresql', 'postgresql_psycopg2', 'sqlite3'

[Ramiro Morales]

Replying to anonymous:

I subclassed a database-backend while waiting for a solution to this ticket to wind up in trunk, as according to ​http://www.djangoproject.com/documentation/settings/#database-engine this should be possible:

Make sure you are really doing what the documentations says, see ​http://code.djangoproject.com/ticket/6676#comment:1. Please don't reply here, take it to django-users if you need further help.

[Malcolm Tredinnick]

For anybody working to bring this to conclusion, a few notes from some discussions Rob Hudson and I have had about this:

There are three separate sub-problems that probably need to be addressed.

1. Manually specifying the sequence name to use.
2. Specifying that ModelA should use the same sequence name as ModelB
3. Perhaps: somehow creating the sequence for case 1.

It's not necessarily compulsory that all are solved, and the original problem description only looks at situation 1, from what I understand. The difficulty there is when does this sequence we are referring to by name get created? It can't be as part of "initial SQL" processing or post_syncdb signal handling, since that's after the table has been created and, presumably, the sequence has to exist before you can refer to it in a table definition. Maybe that's completely out of scope (quite possibly). If not, it's situation 3. Solving that certainly makes testing a lot easier (might well move it from "impossible" into the "possible" category).

The second point, above, seems to be something that might crop up quite naturally here. If you want to share a sequence for some reason, you need to indicate that, but you don't actually know the name of the sequence. Again, maybe out of scope, although it would be nice if it were possible.

[anonymous]

Is there any chance this patch can be integrated into the core?

[1st2be@…]

I chose to take a slightly different approach to this issue, which seems a less intrusive alternative. It limited to one sequence per table, but I doubt that's an issue.

In db/models/options.py:
Added property 'sequence_name'

In postgresql/operations.py

def sequence_currval(self, cursor, sequence):

cursor.execute("SELECT CURRVAL('\"%s\"')" % (sequence,))
return cursor.fetchone()[0]

models/sql/InsertQuery.execute_sql() # last if before catch-all return

if self.model._meta.sequence_name:

return self.connection.ops.sequence_currval(

cursor, self.model._meta.sequence_name)

[HM]

I wound up making my own postgres-backend (based on psycopg2) that asks postgres itself what the sequence is, recursively:

# introspection.py

from django.db.backends.postgresql_psycopg2.base import DatabaseOperations
from django.db.backends.postgresql_psycopg2.introspection import *
from psycopg2 import ProgrammingError

def quote_name(name):
    if name.startswith('"') and name.endswith('"'):
        return name # Quoting once is enough.
    return '"%s"' % name

def _get_direct_sequence_name(cursor, table_name, pk_name):
    "Fetch the sequence name of an auto-incrementing field, if any"
    table_name = quote_name(table_name)
    cursor.execute('''SELECT * FROM pg_get_serial_sequence(%s, %s);''', (table_name, pk_name))
    return cursor.fetchone()[0]

def get_sequence_name(cursor, table_name, pk_name, level=1):
    "Fetch the sequence name of an auto-incrementing field, if any, also tries parents"
    try:
        seq_name = _get_direct_sequence_name(cursor, table_name, pk_name)
        if not seq_name:
            raise ProgrammingError, "Table '%s' had no direct sequence-name" % table_name
        return seq_name
    except ProgrammingError, e:
        cursor.execute('SELECT parent.relname '
            'FROM pg_inherits i '
            'INNER JOIN pg_class parent ON parent.oid = i.inhparent '
            'INNER JOIN pg_class child ON child.oid = i.inhrelid '
            'WHERE child.relname = %s ORDER BY i.inhseqno;', (table_name,))
        if cursor.rowcount < 1 or cursor.rowcount is None:
            raise ProgrammingError, 'No sequence found for pk %s of table %s or parents of table %s' % (pk_name, table_name, table_name)
        # This is safe as the number of rows returned usually will be very few
        rows = [str(row[0]) for row in cursor.fetchall()]
        for parent in rows:
            seq_name = _get_direct_sequence_name(cursor, parent, pk_name)
            if seq_name:
                return seq_name.split('.')[1]
        # XXX: level (recursion-depth) should be a setting?
        if level < 3:
            return get_sequence_name(cursor, parent, pk_name, level+1)
    return None

I think the below is the minimum needed in base.py:

# base.py

# lots of imports here, look at django.db.backends.postgresql_psycopg2.base

class DatabaseOperations(PostgresqlDatabaseOperations):

    def last_insert_id(self, cursor, table_name, pk_name):
        "Try fetching existing sequence, else generate as per standard django"
        seq_name = get_sequence_name(cursor, table_name, pk_name)
        if seq_name is None:
            seq_name = "%s_%s_seq" % (table_name, pk_name)
        cursor.execute("SELECT CURRVAL(%s)", (seq_name,))
        return cursor.fetchone()[0]

class DatabaseWrapper(BaseDatabaseWrapper):
    def __init__(self, *args, **kwargs):
        super(DatabaseWrapper, self).__init__(*args, **kwargs)
        self.ops = DatabaseOperations()

The annoyances with this solution is of course that it is slower than hardcoded sequence-names and that it has to be deployed with django, as a dir in django/db/backends/.

Now, a system that fetched the sequence-names during model-import and stored them in the models, that would maybe be the best solution (for a legacy database like what I'm working with anyway).

[shmengie]

HM I like the elegance of your solution, but the potential performance repercussion will likely keep it out of the codebase.

The Meta.sequence_name = 'my_cust_seq' has minimal performance impact (one python if statement) per InsertQuery. Legacy integrators will not be concerned with table creation, but will appreciate being able to integrate their schema into django. The Meta.sequence_name would need to be documented.

Pros:

Eases legacy integration
Adds feature custom sequence_name ability

Cons:

Does not address sequence creation issues
minimal performance impact (more a pro than con)
minimal additional code in base

[HM]

Partially fixed by #8901.

[Jacob]

#13295 has a better patch; marking duplicate of that one.