[Django] #21453: Set autocommit mode before calling init_connection

Django

unread,

Nov 16, 2013, 4:32:54 PM11/16/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Database | Keywords:
layer (models, ORM) | Has patch: 0
Severity: Normal | Needs tests: 0
Triage Stage: | Easy pickings: 0
Unreviewed |
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
I just helped an user on IRC who attempted a fairly reasonable database
backend customization:
{{{
from django.db.backends.postgresql_psycopg2 import base

class DatabaseWrapper(base.DatabaseWrapper):
def init_connection_state(self):
super(DatabaseWrapper, self).init_connection_state()
self.connection.cursor().execute('SET search_path TO something')
}}}

Unfortunately that fails with an ugly `ProgrammingError: autocommit cannot
be used inside a transaction` and the traceback is very confusing.

The root cause of this problem is that the code above starts a
transaction, and when Django later tries to switch to autocommit,
PostgreSQL complains.

Would it make sense to switch to autocommit before calling
`init_connection_state`? This is just a matter of swapping two lines in
`BaseDatabaseWrapper.connect`, and reviewing carefully the code in
`init_connection_state` to ensure it still works as expected. (Hint: it
doesn't, see #21452.)

Currently `init_connection_state` executes in PEP-249 mode (= implicit
transactions = no autocommit), while everything else executes in
autocommit mode ''(unless you set AUTOCOMMIT to False in the database
settings, but I'm really mentioning that for the sake of completeness, I
don't expect anyone to do this)''. That's quite surprising and probably a
good reason for making this change. Of course, there may be even better
reasons for NOT making this change that I haven't thought of.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Nov 16, 2013, 4:36:26 PM11/16/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6

* cc: w2lkm2n@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:1>

Django

unread,

Nov 16, 2013, 5:25:34 PM11/16/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6

Comment (by akaariai):

I think we should handle this in #21452. The short version is that after
init_connection_state autocommit will always be off, but it does get re-
enabled in BaseDatabaseWrapper.connect(). More details in #21452.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:2>

Django

unread,

Nov 16, 2013, 6:30:39 PM11/16/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6

Comment (by aaugustin):

I'd rather do it the other way round: decide whether it makes most sense
to call `init_connection_state` in PEP-249 mode or in the mode selected in
the database settings, and then write a suitable patch for #21452.

I'm afraid `init_connection_state` has to be aware of transactions anyway.
For instance, for setting the time zone as discussed in #21452:
- in PEP-249 mode, we must either switch to autocommit temporarily or
commit after setting the time zone (the latter seems more straightforward,
why don't we do it that way currently?)
- in the mode selected in the database settings, if that mode is
autocommit, we're fine, if it isn't, we have to do the same as above.

Switching makes it easier to override `init_connection_state` and make it
work in autocommit, but harder when autocommit is disabled. Given that
very very few people will disable autocommit, it could still be a good
tradeoff.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:3>

Django

unread,

Nov 17, 2013, 5:33:23 AM11/17/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6

Comment (by akaariai):

Maybe we could even go for always run init_connection_state in autocommit
mode, switch to settings mode after init_connection_state is done? This
way init_connection_state doesn't need to be aware of transactions.

As for why wasn't commit used for timezone set, I don't recall if that was
ever tried.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:4>

Django

unread,

Nov 17, 2013, 7:04:12 AM11/17/13

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6

* stage: Unreviewed => Accepted

Comment:

Marking this as accepted - this seems like a good idea to me.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:5>

Django

unread,

Jan 12, 2014, 12:54:24 PM1/12/14

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------

Reporter: aaugustin | Owner: aaugustin
Type: | Status: assigned
Cleanup/optimization | Version: 1.6

Changes (by aaugustin):

* status: new => assigned
* owner: nobody => aaugustin

Comment:

Finally I handled #21452 first because I wanted to backport it to 1.6.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:6>

Django

unread,

Jan 12, 2014, 2:29:53 PM1/12/14

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------

Reporter: aaugustin | Owner: aaugustin
Type: | Status: assigned
Cleanup/optimization | Version: 1.6

Comment (by aaugustin):

Being aware of transactions means ending `init_connection_state` with:

{{{
# Ensure all changes are preserved even when AUTOCOMMIT is False.
if not self.get_autocommit():
self.commit()
}}}

I think that's fine.

I'm going to run `init_connection_state` in the mode prescribed by the
settings because it's the most reasonable expectation.

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:7>

Django

unread,

Jan 12, 2014, 3:11:34 PM1/12/14

to django-...@googlegroups.com

#21453: Set autocommit mode before calling init_connection_state
-------------------------------------+-------------------------------------

Reporter: aaugustin | Owner: aaugustin
Type: | Status: closed
Cleanup/optimization | Version: 1.6
Component: Database layer | Resolution: fixed

Changes (by Aymeric Augustin <aymeric.augustin@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"fbe1abac4af3a7fc138bd176471e36acb1070a58"]:
{{{
#!CommitTicketReference repository=""
revision="fbe1abac4af3a7fc138bd176471e36acb1070a58"
Fixed #21453 -- Enabled autocommit before calling init_connection_state.

Also ensured the transaction state is clean on Oracle while I was there.

This change cannot be backported to 1.6 because it's
backwards-incompatible for custom database backends.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21453#comment:8>

Reply all

Reply to author

Forward

[Django] #21453: Set autocommit mode before calling init_connection_state

Django

Django

Django

Django

Django

Django

Django

Django

Django