Fix(?) model insert handling when autcommit=True for PostgreSQL <= 8.1

[Malcolm Tredinnick]

Need to wrap some isolation level changes around inserts when using older PostgreSQL versions so that we can reliably return the last insert ID. This is necessary because we use that value to set the primary key attribute on the model instance and there's potentially no other way to get the information for some models (with no other unique constraints).

When this is fixed, settings.DATABASE_OPTIONS[autocommit] = True can be an option again for those server versions.

[Malcolm Tredinnick]

This doesn't have to be a "beta" fix. It's a bug-fix on the original feature. If we get time to do it before 1.1, that's cool. If not, it's a documentation note to say it doesn't work on those versions yet. Somebody trying to squeeze maximum performance from PostgreSQL at this level should consider using the latest version anyway.

[Malcolm Tredinnick]

Can wait until later. Right now, an error is correctly reported. I want to fix it properly so that everybody doesn't have to pay all the version checking penalties we currently do for this sort of thing (some kind of DATABASE_OPTIONS specification of version number, probably). Anyway, something to add for 1.2.

[Richard Davies <richard.davies@…>]

Remove unnecessary test - it works already!

[Richard Davies <richard.davies@…>]

It turns out that the current code already works on PostgreSQL <= 8.1. I'm attaching a patch which simply removes the check (and some documentation of the limitation). This means that we'll be able to get database-native autocommit into 1.1. for all versions of PostgreSQL - good!

The secret is that django/db/backends/postgresql/operations.py already defines last_insert_id() in terms of 'select currval' which is defined to be a session-local value (see ​http://www.postgresql.org/docs/7.3/static/functions-sequence.html or any other version).

When django/db/models/sql/subqueries.py defines InsertQuery.execute_sql(), the version which uses return last_insert_id() is already session-safe, even in database-native autocommit mode without a wrapping transaction. The version which uses the 'returning' syntax on Postgresql >= 8.2 is simply more efficient, not more session-safe.

[Richard Davies <richard.davies@…>]

Wider clean up of tests in this area

[Richard Davies <richard.davies@…>]

The last patch leaves the 'if version >= 8.2' rather awkwardly inside an 'if uses_autocommit'.

I don't think that outer 'if' statement is necessary, even in light of #10467 comment 4 and #10509, so here's a version which removes it, and hence allows the 'returning' syntax to be used whenever version >= 8.2, regardless of uses_autocommit.

I think that [10065] was confused between two things:

• setting 'can_return_id_from_insert=True' inside _cursor() rather than by default, which is necessary and the point of #10509

• only doing this if uses_autocommit, which was overcautious and unnecessary

[Russell Keith-Magee]

Like Malcolm said, this can wait until v1.2. We won't ever release v1.1 if we keep adding tickets that aren't essential.

[Richard Davies <richard.davies@…>]

Adding to the 1.2 milestone - hopefully we can get this completed patch committed this time.

[Richard Davies <richard.davies@…>]

I have just checked that this patch still applies and works with the current SVN head.

I mentioned it in the 1.2 features discussion, and it was categorized under "Small, or Just Bugs" as a small non-controversial feature addition.

As such, I'd like to see it checked in before Django 1.2 beta, please.

[James Bennett]

Patch hasn't been updated since before 1.1, so I'd be wary of checking it in now for 1.2. Also, the comments above indicate this is more a feature addition, and unfortunately 1.2's feature-frozen.

[Adam Nelson]

Looks like this won't make it into milestone:1.3 either?

[Adam Nelson]

PostgreSQL 8.1 is ​no longer supported by the Postgres community. I think this ticket should simply be closed at this point.