Re: autocommit property broken with MySQL

Matthew Bogosian <[email protected]> Tue, 16 Aug 2005 11:22:54 -0700
Newsgroups gmane.comp.web.skunkweb
Message-ID <[email protected]>
On Aug 10, 2005, at 11:06, Matthew Bogosian wrote:

> ...
>
> It seems like this is a particularly hairy issue since there doesn't 
> appear to be any standard regarding its implementation. MySQL makes 
> things even more confusing since non InnoDB tables behave as if 
> autocommit was on even if it's not. I guess that's a detail which 
> should be taken into account by the database designer (i.e., if you 
> depend on transactionality and are design an application for use with 
> several database back-ends, then you should probably be aware of the 
> idiosyncrasies of each).
>
> One solution might be to get rid of autocommit and add something to 
> the alias definition which would be zero or more statements which 
> would be executed on every new connection immediately after it was 
> created. For example:
>
>> conn_args = \
>> {
>>     'db': 'mydb',
>>     'user': 'myuser',
>>     'passwd': 'mypass',
>> }
>> init_sql = \
>> (
>>     'SET AUTOCOMMIT = 0',
>>     ...
>> )
>> pydo.dbi.initAlias('mydb', 'mysql', conn_args, initSql = init_sql)
>
> That way, one could do any appropriate initialization without having 
> to sub-class an existing PyDO DBI implementation. New connections 
> which share the alias would all have the same initialization. It 
> doesn't address the abstract the want/need to see if a particular 
> connection has the autocommit attribute set, but I'm wondering if 
> that's such a bad thing, since it seems to be such a non standard 
> thing anyway.
>
> ...

Attached is a patch (made from SVN #1628) to implement the proposal 
alluded to above. This patch gets rid of the autocommit property 
altogether and provides a new parameter to DB aliases called 'initSQL' 
which can be a string or a sequence of strings. If present, the 
statement(s) will be executed (in sequence) immediately after 
connection creation.

At least that was the intention. :-)



To apply the patch:

	svn co svn://svn.berlios.de/skunkweb/trunk/PyDO PyDO-SVN
	cd PyDO-SVN
	patch -p0 </path/to/init_sql.diff

To use:

	pydo.dbi.initAlias(..., initSQL = 'SET AUTOCOMMIT = 0')

	# Or...

	init_sql = \
	(
		'SET AUTOCOMMIT = 0',
		'SET MAX_JOIN_SIZE = %d' % 2 ** 16 - 1,
	)
	pydo.dbi.initAlias(..., initSQL = init_sql)

I don't know if this is useful, but I figured it couldn't hurt to 
try.... One improvement one might make to this patch is to allow for 
something like this:

	pydo.dbi.initAlias(...,
		initSQL = 'SET AUTOCOMMIT = %s',
		initSQLArgs = 0)

	# Or...

	init_sql = \
	(
		'SET AUTOCOMMIT = %s',
		'SET MAX_JOIN_SIZE = %s',
		"""
		INSERT INTO logins
		(login_epoch_secs, login_user)
		VALUES (%s, %s)'
		""",
		...
	)
	init_sql_args = \
     (
		0,
		2 ** 16 - 1,
		(time.time(), 'floppy'),
		...
	)
	pydo.dbi.initAlias(...,
		initSQL = init_sql,
		initSQLArgs = init_sql_args)

This should be a relatively simple change to make. If there's any 
interest in the existing patch, I'll see what I can do about 
implementing the above.

	-- Matt
init_sql.diff (application/octet-stream, 4.7 KB)
Index: src/pydo/dbi.py
===================================================================
--- src/pydo/dbi.py	(revision 1631)
+++ src/pydo/dbi.py	(working copy)
@@ -2,6 +2,7 @@
 from threading import Lock, local
 from collections import deque
 import time
+import types
 from pydo.log import *
 from pydo.operators import BindingConverter
 from pydo.exceptions import PyDOError
@@ -50,14 +51,6 @@
     def _initExceptions(self):
         self.exceptions=dict((e, getattr(self.dbapiModule, e)) for e in exception_names)
 
-    def autocommit():
-        def fget(self):
-            return self.conn.autocommit
-        def fset(self, val):
-            self.conn.autocommit=val
-        return fget, fset, None, None
-    autocommit=property(*autocommit())
-
     def conn():
         def fget(self):
             try:
@@ -133,9 +126,6 @@
             return c.rowcount
         res=self._convertResultSet(c.description, resultset, qualified)
         c.close()
-        if self.autocommit and self.pool:
-            # release connection
-            del self.conn
         return res
     
     @staticmethod
@@ -283,6 +273,16 @@
             raise ValueError, "alias %s not recognized" % alias
         if not conndata.has_key('connection'):
             res=_connect(**conndata)
+            if conndata.has_key('initSQL'):
+                if type(conndata['initSQL'] in types.StringTypes):
+                    conndata['initSQL'] = ( conndata['initSQL'], )
+                for initSQL in conndata['initSQL']:
+                    try:
+                        res.execute(initSQL)
+                    except:
+                        res.rollback()
+                        del res
+                        raise
             conndata['connection']=res
             return res
         return conndata['connection']
@@ -419,14 +419,8 @@
 
     def onRelease(self, realConn):
         """anything you want to do to a connection when it is returned
-        (default: rollback if not autocommit)"""
-        try:
-            if not realConn.autocommit:
-                realConn.rollback()
-        except:
-            # psycopg 2 doesn't seems to support autocommit, which
-            # seems bogus to me...
-            pass
+        (default: rollback)"""
+        realConn.rollback()
 
         
 
Index: src/pydo/drivers/mssqlconn.py
===================================================================
--- src/pydo/drivers/mssqlconn.py	(revision 1631)
+++ src/pydo/drivers/mssqlconn.py	(working copy)
@@ -86,16 +86,6 @@
                                      adodbapi,
                                      pool,
                                      verbose)
-      #
-      # The DBI code seems to be looking for an
-      #  autocommit attribute of the underlying
-      #  db driver, even though the spec doesn't
-      #  seem to suggest it's mandatory. Since
-      #  I think SQL Server does do what the
-      #  code is expecting from autocommit,
-      #  turn the attribute on here.
-      #
-      self.conn.autocommit = True
 
    def getConverter(self):
       return MssqlConverter(self.paramstyle)
Index: src/pydo/drivers/psycopgconn.py
===================================================================
--- src/pydo/drivers/psycopgconn.py	(revision 1631)
+++ src/pydo/drivers/psycopgconn.py	(working copy)
@@ -115,32 +115,7 @@
                                         psycopg,
                                         pool,
                                         verbose)
-       if psycopg_version<2:
-           # try to keep state
-           self._autocommit=None
 
-    if psycopg_version==2:
-        def autocommit():
-            def fget(self):
-                return self.conn.isolation_level==0
-            def fset(self, val):
-                self.conn.set_isolation_level(not val)
-            return fget, fset, None, None
-        autocommit=property(*autocommit())
-
-    else:
-        def autocommit():
-            def fget(self):
-                return self._autocommit
-            def fset(self, val):
-                self._autocommit=val
-                if val:
-                    self.conn.autocommit()
-                else:
-                    self.conn.autocommit(0)
-            return fget, fset, None, None
-        autocommit=property(*autocommit())
-    
     def getConverter(self):
         return PsycopgConverter(self.paramstyle)
     
Index: src/pydo/drivers/oracleconn.py
===================================================================
--- src/pydo/drivers/oracleconn.py	(revision 1631)
+++ src/pydo/drivers/oracleconn.py	(working copy)
@@ -10,8 +10,6 @@
 
     paramstyle = 'named'
     
-    autocommit = None
-    
     def __init__(self, connectArgs, pool=None, verbose=False):
        if pool and not hasattr(pool, 'connect'):
           pool = ConnectionPool()
PGP.sig (application/pgp-signature, 186 B) - not displayed