Set autocommit mode before calling init_connection_state

[Aymeric Augustin]

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.

[Anssi Kääriäinen]

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.

[Aymeric Augustin]

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.

[Anssi Kääriäinen]

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.

[Anssi Kääriäinen]

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

[Aymeric Augustin]

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

[Aymeric Augustin]

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.

[Aymeric Augustin <aymeric.augustin@…>]

In 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.