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 , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 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 , 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 , 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 , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Version: | 1.3-rc1 → 1.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.
Some questions so we can evaluate if we should undo that change of strategy for obtaining the last inserted row ID:
1.3-rc1
as the Django version number in the Version ticket field?