#12180 closed Bug (fixed)
ProgrammingError thrown with autocommit: True if first query on PostgreSQL >= 8.2 is an INSERT
Reported by: | Christophe Pettus | Owned by: | Christophe Pettus |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | brett@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
There is a bug in the handling of InsertQuery.connection.features.can_return_id_from_insert, which is causing Django 1.1.1 to throw a ProgrammingError exception when inserting a new object/record into the database, using PostgreSQL 8.4.1, using psycopg2, if that INSERT is the first thing done by a view on a particular connection to the database, when DATABASE_OPTIONS autocommit: True is set.
The details are:
- Django uses InsertQuery.connection.features.can_return_id_from_insert to decide whether or not append a RETURNING clause to the INSERT, so that it can get the primary key of a newly-inserted object (db/models/sql/subqueries.py, lines 311-315).
- That flag is set in the _cursor method of DatabaseWrapper (db/backends/postgresql_psycopg2/base.py, lines 106-121), but the cursor hasn't been created yet in step #1, so can_return_id_from_insert is always False.
- But Django then issues the query (creating the cursor, and correctly setting can_return_id_from_insert to True), and thus expects a return value to come back the INSERT statement, but since the RETURNING clause wasn't added in step #1, it throws a ProgrammingError exception when it tries to get the expected return value (db/models/sql/subqueries.py, lines 323-324).
This appears to be a bad interaction with autocommit: True, since the code is clearly expecting a transaction to already have been opened at this point, which would have set the version information and the can_return_id_from_insert to True.
Attachments (2)
Change History (20)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Looking at #10467, I now get it: autocommit = True is being used as a (checked) assertion that the PG version is >= 8.2, so that the initially-constructed SQL on an INSERT doesn't include a RETURNING for PG versions <8.2... sadly, it seems to have just moved the bug to a different place.
by , 15 years ago
Attachment: | 12180.diff added |
---|
comment:3 by , 15 years ago
I've attached a proposed patch. The patch just sets "self.features.can_return_id_from_insert" based on the autocommit setting. This is, of course, a hack (although not really any more hacky than the current code). This allows SQL-generation operations before cursor creation to get a correct value for self.features.can_return_id_from_insert. Of course, autocommit could be lying and the version of PostgreSQL could be such that autocommit (and self.features.can_return_id_from_insert) don't apply, but this will be checked before any queries are actually sent to the db.
comment:4 by , 15 years ago
I've attached another patch, this one with some redundant code removed. This doesn't solve the full issue described in #10509, but it does not reopen the prior bug of sending an INSERT involving a RETURNING to a version of PostgreSQL < 8.2, as the autocommit will cause the version to be checked, and an error raised if it's not >=8.2.
comment:5 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 15 years ago
Has patch: | set |
---|
comment:7 by , 15 years ago
milestone: | → 1.2 |
---|
comment:8 by , 15 years ago
Needs tests: | set |
---|
comment:9 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:11 by , 15 years ago
Cc: | added |
---|
comment:12 by , 15 years ago
I'm not sure why this isn't considered critical for 1.2, but I'd at least like to note that Xof's patch works well for me (as this was a blocker for our 1.2 deploy).
comment:14 by , 14 years ago
milestone: | 1.3 |
---|---|
Severity: | → Normal |
Type: | → Bug |
comment:16 by , 13 years ago
Easy pickings: | unset |
---|---|
Version: | 1.1 → 1.3 |
I've been using this patch really solves the problem. I wonder, when will be included in an official version?
comment:18 by , 13 years ago
UI/UX: | unset |
---|
Note that, for anyone needing this fix on Django 1.3, it appears to be possible to implement the required functionality from this patch by adding the following to your urls.py (with a hefty comment explaining why, of course):
from django.db.backends.postgresql_psycopg2.base import DatabaseFeatures DatabaseFeatures.can_return_id_from_insert = True
Looking at it a bit further, I'm not quite sure why can_return_id_from_insert depends on uses_autocommit.