Opened 13 years ago

Closed 13 years ago

#17284 closed Bug (wontfix)

last_insert_id in django.db.backends.postgresql.operations.py

Reported by: EErlo Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords: postgres 8.1.23, ORM save
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The new version of function last_insert_id uses the "new" postgres( >= version 8) method to get the sequence of a table (pg_get_serial_sequence), but in postgres 8.1.23(so far) this method isn't 100% good.
In some rare situations, postgres returns NULL, returning None to a saved new model pk, making lots of problems, even on Django Admin Interface, when you need the new-created pk to do something after call the save() method, as the Admin Interface Auth User does. I have made a treatment in django.db.backends.postgresql.operations.py, in the last_insert_id function:

    def last_insert_id(self, cursor, table_name, pk_name):
        # Use pg_get_serial_sequence to get the underlying sequence name
        # from the table name and column name (available since PostgreSQL 8)
        cursor.execute("SELECT CURRVAL(pg_get_serial_sequence('%s','%s'))" % (
            self.quote_name(table_name), pk_name))
        to_return = cursor.fetchone()[0]
        if not to_return or to_return == 'None':

            cursor.execute("select substr(column_default,10,position('::regclass' in column_default)-11) from information_schema.columns where table_name = '%s' and column_name = '%s'" % (table_name, pk_name))
            seq_name = cursor.fetchone()[0]
            cursor.execute("SELECT CURRVAL('%s')"%(seq_name))
            to_return = cursor.fetchone()[0]
        return to_return

I wait for your answer to know if it will be added to the django next release.

Change History (5)

comment:1 by Ramiro Morales, 13 years ago

Triage Stage: UnreviewedAccepted

Some questions so we can evaluate if we should undo that change of strategy for obtainig last interted row ID:

  • Why have you selected 1.3-rc1 as the Django version number in the Version ticket field?
  • What exact revision of Django are you using?
  • What version of psycopg2 are you using?
Version 0, edited 13 years ago by Ramiro Morales (next)

comment:2 by Anssi Kääriäinen, 13 years ago

An assert that something is going to be returned would be a good thing.

Also, that if branch sequence lookup is slow. On my system the lookup takes 10ms, while the actual insert takes 0.5ms. This is probably nothing to worry about, since the if branch is just a fallback.

comment:3 by Ramiro Morales, 13 years ago

Wait, I had misread this report. This isn't about the changes we introduced for Django 1.4 (the ones that raise the minimum supported Postgres version to 8.2 and switches to always use "INSERT... RETURNING..." for this task) so please disregard ther first paragraph of my comment.

This is about using "SELECT CURRVAL(pg_get_serial_sequence('%s','%s'))" % (table_name, pk_name)

It is strange that this is the first report of this kind of behavior since we introduced usage of pg_get_serial_sequence() in r13328 back in June 2010.

I don't know if a change like the proposed would have a chance to get in a maintenance branch like 1.3.x especially considering that PostgreSQ 8.1 has been deprecated upstream since Nov 2010. Maybe you can remove the usage of pg_get_serial_sequence() reverting back to the previous hard-coded "SELECT CURRVAL('\"%s_%s_seq\"')" % (table_name, pk_name) in your local copy of Django.

It still would be great if you could post the information we asked for.

comment:4 by eduardo.erlo@…, 13 years ago

My Django Version: 1.3.1 final ..

django.VERSION = (1, 3, 1, 'final', 0)..

psycopg2.version = '2.2.2 (dt dec ext pq3)'

comment:5 by Ramiro Morales, 13 years ago

Resolution: wontfix
Status: newclosed
Version: 1.3-rc11.3

wontfixig for the reasons exposed in comment:3: There is no chance to get a change like the proposed one in a hypothetical 1.3.2 release (these releases only containg fixes for release-blocking critical or security issues) and we've dropped support for Postgres older than 8.2 in 1.4.

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