#21452 closed Bug (fixed)
Connections may be set to autocommit under PostgreSQL when AUTOCOMMIT is set to False.
Reported by: | Aymeric Augustin | Owned by: | Aymeric Augustin |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.6 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django/db/backends/postgresql_psycopg2/base.py contains this code:
if conn_tz != tz: # Set the time zone in autocommit mode (see #17062) self.set_autocommit(True) self.connection.cursor().execute( self.ops.set_time_zone_sql(), [tz])
This will leave the connection in autocommit mode regardless of the AUTOCOMMIT setting for the database connection.
I haven't confirmed the bug; this report is just based on my understanding of the code.
Change History (7)
comment:1 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | master → 1.6 |
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Yes, now I remember psycopg2
's APIs for isolation levels interact with autocommit. Django even used to activate autocommit through set_isolation_level
.
I think I carefully refactored this area without changing the behavior when I wrote the new transaction management, but mistakes are always possible.
I'm adding this to my TODO list for the sprints in Berlin next week.
comment:3 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:5 by , 11 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Reopening because the test fails under PostGIS:
comment:6 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Actually I think the code happens to (almost) work both when autocommit is on or off.
If autocommit is on in settings, then set_autocommit(True) gets called from connect() method of
backends/__init__.py:BaseDatabaseWrapper
. End result is that autocommit is enabled.If autocommit is off in settings, then self.connection.set_isolation_level(self.isolation_level) gets called as last thing in init_connection_state(). This sets autocommit off. However, if self.set_autocommit(True) is called from the code block mentioned in ticket's description, then autocommit will be off for the connection, but Django will think the connection is in autocommit mode (self.autocommit is True).
Also, after init_connection_state() but before set_autocommit() is called from BaseDatabaseWrapper's connect() the connection will be in non-autocommit mode. This is also why #21453's use case doesn't work as expected.
I think we need to clean up how isolation level changes are done for new connections, and also backpatch to 1.6.x.