]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
- Simplified the check for ResultProxy "autoclose without results"
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 4 Nov 2008 17:28:26 +0000 (17:28 +0000)
committerMike Bayer <mike_mp@zzzcomputing.com>
Tue, 4 Nov 2008 17:28:26 +0000 (17:28 +0000)
to be based solely on presence of cursor.description.
All the regexp-based guessing about statements returning rows
has been removed [ticket:1212].

CHANGES
lib/sqlalchemy/databases/firebird.py
lib/sqlalchemy/databases/mssql.py
lib/sqlalchemy/databases/mysql.py
lib/sqlalchemy/databases/oracle.py
lib/sqlalchemy/databases/postgres.py
lib/sqlalchemy/databases/sqlite.py
lib/sqlalchemy/engine/base.py
lib/sqlalchemy/engine/default.py
test/profiling/zoomark.py
test/profiling/zoomark_orm.py

diff --git a/CHANGES b/CHANGES
index 8fbdbb5da744f4b26edcedc4bae0918374d6fcfe..2b61127a51b17034f3bc39716bfa76d8d17e8c1c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -58,6 +58,11 @@ CHANGES
       upon disposal to keep it outside of cyclic garbage collection.
 
 - sql
+    - Simplified the check for ResultProxy "autoclose without results"
+      to be based solely on presence of cursor.description.   
+      All the regexp-based guessing about statements returning rows 
+      has been removed [ticket:1212].
+      
     - Further simplified SELECT compilation and its relationship to
       result row processing.
 
@@ -96,9 +101,6 @@ CHANGES
     - No longer expects include_columns in table reflection to be
       lower case.
 
-    - added 'EXPLAIN' to the list of 'returns rows', but this 
-      issue will be addressed more fully by [ticket:1212].
-      
 - misc
     - util.flatten_iterator() func doesn't interpret strings with
       __iter__() methods as iterators, such as in pypy [ticket:1077].
index ccd4db3c77cb4e7203c33fcd7ee12a61ab5a30fc..7c7d4793be81fef75acfa7b18d597877ff7db72b 100644 (file)
@@ -261,45 +261,10 @@ ischema_names = {
        'BLOB': lambda r: r['stype']==1 and FBText() or FBBinary()
       }
 
-
-SELECT_RE = re.compile(
-    r'\s*(?:SELECT|(UPDATE|INSERT|DELETE))',
-    re.I | re.UNICODE)
-
-RETURNING_RE = re.compile(
-    'RETURNING',
-    re.I | re.UNICODE)
-
-# This finds if the RETURNING is not inside a quoted/commented values. Handles string literals,
-# quoted identifiers, dollar quotes, SQL comments and C style multiline comments. This does not
-# handle correctly nested C style quotes, lets hope no one does the following:
-# UPDATE tbl SET x=y /* foo /* bar */ RETURNING */
-RETURNING_QUOTED_RE = re.compile(
-    """\s*(?:UPDATE|INSERT|DELETE)\s
-        (?: # handle quoted and commented tokens separately
-            [^'"$/-] # non quote/comment character
-            | -(?!-) # a dash that does not begin a comment
-            | /(?!\*) # a slash that does not begin a comment
-            | "(?:[^"]|"")*" # quoted literal
-            | '(?:[^']|'')*' # quoted string
-            | --[^\\n]*(?=\\n) # SQL comment, leave out line ending as that counts as whitespace
-                               # for the returning token
-            | /\*([^*]|\*(?!/))*\*/ # C style comment, doesn't handle nesting
-        )*
-        \sRETURNING\s""", re.I | re.UNICODE | re.VERBOSE)
-
 RETURNING_KW_NAME = 'firebird_returning'
 
 class FBExecutionContext(default.DefaultExecutionContext):
-    def returns_rows_text(self, statement):
-        m = SELECT_RE.match(statement)
-        return m and (not m.group(1) or (RETURNING_RE.search(statement)
-                                         and RETURNING_QUOTED_RE.match(statement)))
-
-    def returns_rows_compiled(self, compiled):
-        return (isinstance(compiled.statement, sql.expression.Selectable) or
-                ((compiled.isupdate or compiled.isinsert or compiled.isdelete) and
-                 RETURNING_KW_NAME in compiled.statement.kwargs))
+    pass
 
 
 class FBDialect(default.DefaultDialect):
index e6da0d3fc1c55df0a2f78895382b3933f61a303f..f86a95548258b5f2c270b54b4b680324a333ddce 100644 (file)
@@ -337,12 +337,6 @@ class MSSQLExecutionContext(default.DefaultExecutionContext):
                 self._last_inserted_ids = [int(row[0])] + self._last_inserted_ids[1:]
         super(MSSQLExecutionContext, self).post_exec()
 
-    _ms_is_select = re.compile(r'\s*(?:SELECT|sp_columns|EXEC)',
-                               re.I | re.UNICODE)
-
-    def returns_rows_text(self, statement):
-        return self._ms_is_select.match(statement) is not None
-
 
 class MSSQLExecutionContext_pyodbc (MSSQLExecutionContext):
     def pre_exec(self):
index b03ee5764c93635e33c7ad0b3b7e825a93bad5ba..3dbb1797d5fc6b2765915d2b577098cab0a0eddb 100644 (file)
@@ -220,9 +220,6 @@ RESERVED_WORDS = set(
 AUTOCOMMIT_RE = re.compile(
     r'\s*(?:UPDATE|INSERT|CREATE|DELETE|DROP|ALTER|LOAD +DATA|REPLACE)',
     re.I | re.UNICODE)
-SELECT_RE = re.compile(
-    r'\s*(?:SELECT|SHOW|DESCRIBE|XA RECOVER|CALL|EXPLAIN)',
-    re.I | re.UNICODE)
 SET_RE = re.compile(
     r'\s*SET\s+(?:(?:GLOBAL|SESSION)\s+)?\w',
     re.I | re.UNICODE)
@@ -1463,9 +1460,6 @@ class MySQLExecutionContext(default.DefaultExecutionContext):
             # which is probably a programming error anyhow.
             self.connection.info.pop(('mysql', 'charset'), None)
 
-    def returns_rows_text(self, statement):
-        return SELECT_RE.match(statement)
-
     def should_autocommit_text(self, statement):
         return AUTOCOMMIT_RE.match(statement)
 
index ffd52c38c6f2971fbeb36f45ed27bba3adc5e993..15b18b1cb04f8066656447f4212c0ac5b3baf75f 100644 (file)
@@ -123,8 +123,6 @@ from sqlalchemy.sql import operators as sql_operators, functions as sql_function
 from sqlalchemy import types as sqltypes
 
 
-SELECT_REGEXP = re.compile(r'(\s*/\*\+.*?\*/)?\s*SELECT', re.I | re.UNICODE)
-
 class OracleNumeric(sqltypes.Numeric):
     def get_col_spec(self):
         if self.precision is None:
@@ -321,9 +319,6 @@ class OracleExecutionContext(default.DefaultExecutionContext):
                     self.out_parameters[name] = self.cursor.var(dbtype)
                     self.parameters[0][name] = self.out_parameters[name]
 
-    def returns_rows_text(self, statement):
-        return SELECT_REGEXP.match(statement)
-
     def create_cursor(self):
         c = self._connection.connection.cursor()
         if self.dialect.arraysize:
index c8abeb6db7bdf8e5a0f27887a25af189d49f8325..69fad230dda4a98ec2fa7d7dc01399fb1ef04304 100644 (file)
@@ -225,60 +225,25 @@ ischema_names = {
     'interval':PGInterval,
 }
 
+# TODO: filter out 'FOR UPDATE' statements
 SERVER_SIDE_CURSOR_RE = re.compile(
     r'\s*SELECT',
     re.I | re.UNICODE)
 
-SELECT_RE = re.compile(
-    r'\s*(?:SELECT|FETCH|(UPDATE|INSERT))',
-    re.I | re.UNICODE)
-
-RETURNING_RE = re.compile(
-    'RETURNING',
-    re.I | re.UNICODE)
-
-# This finds if the RETURNING is not inside a quoted/commented values. Handles string literals,
-# quoted identifiers, dollar quotes, SQL comments and C style multiline comments. This does not
-# handle correctly nested C style quotes, lets hope no one does the following:
-# UPDATE tbl SET x=y /* foo /* bar */ RETURNING */
-RETURNING_QUOTED_RE = re.compile(
-    """\s*(?:UPDATE|INSERT)\s
-        (?: # handle quoted and commented tokens separately
-            [^'"$/-] # non quote/comment character
-            | -(?!-) # a dash that does not begin a comment
-            | /(?!\*) # a slash that does not begin a comment
-            | "(?:[^"]|"")*" # quoted literal
-            | '(?:[^']|'')*' # quoted string
-            | \$(?P<dquote>[^$]*)\$.*?\$(?P=dquote)\$ # dollar quotes
-            | --[^\\n]*(?=\\n) # SQL comment, leave out line ending as that counts as whitespace
-                            # for the returning token
-            | /\*([^*]|\*(?!/))*\*/ # C style comment, doesn't handle nesting
-        )*
-        \sRETURNING\s""", re.I | re.UNICODE | re.VERBOSE)
-
 class PGExecutionContext(default.DefaultExecutionContext):
-    def returns_rows_text(self, statement):
-        m = SELECT_RE.match(statement)
-        return m and (not m.group(1) or (RETURNING_RE.search(statement)
-           and RETURNING_QUOTED_RE.match(statement)))
-
-    def returns_rows_compiled(self, compiled):
-        return isinstance(compiled.statement, expression.Selectable) or \
-            (
-                (compiled.isupdate or compiled.isinsert) and "postgres_returning" in compiled.statement.kwargs
-            )
-
     def create_cursor(self):
-        self.__is_server_side = \
+        # TODO: coverage for server side cursors + select.for_update()
+        is_server_side = \
             self.dialect.server_side_cursors and \
-            ((self.compiled and isinstance(self.compiled.statement, expression.Selectable)) \
+            ((self.compiled and isinstance(self.compiled.statement, expression.Selectable) and not self.compiled.statement.for_update) \
             or \
             (
                 (not self.compiled or isinstance(self.compiled.statement, expression._TextClause)) 
                 and self.statement and SERVER_SIDE_CURSOR_RE.match(self.statement))
             )
 
-        if self.__is_server_side:
+        self.__is_server_side = is_server_side
+        if is_server_side:
             # use server-side cursors:
             # http://lists.initd.org/pipermail/psycopg/2007-January/005251.html
             ident = "c_%s_%s" % (hex(id(self))[2:], hex(random.randint(0, 65535))[2:])
index b3de98100ccce1a80c0a72a6468eab907ca07572..fd35f747fa4e0a2d910fbe8480a046f2526d9e80 100644 (file)
@@ -14,8 +14,6 @@ import sqlalchemy.util as util
 from sqlalchemy.sql import compiler, functions as sql_functions
 from types import NoneType
 
-SELECT_REGEXP = re.compile(r'\s*(?:SELECT|PRAGMA)', re.I | re.UNICODE)
-
 class SLNumeric(sqltypes.Numeric):
     def bind_processor(self, dialect):
         type_ = self.asdecimal and str or float
@@ -234,9 +232,6 @@ class SQLiteExecutionContext(default.DefaultExecutionContext):
             if not len(self._last_inserted_ids) or self._last_inserted_ids[0] is None:
                 self._last_inserted_ids = [self.cursor.lastrowid] + self._last_inserted_ids[1:]
 
-    def returns_rows_text(self, statement):
-        return SELECT_REGEXP.match(statement)
-
 class SQLiteDialect(default.DefaultDialect):
     name = 'sqlite'
     supports_alter = False
index bd0f15ecdf5bb4825fa7a7667b06e1716ca287bd..b810f5f67b57c18e58c64504e869e1587043ffce 100644 (file)
@@ -305,9 +305,6 @@ class ExecutionContext(object):
     should_autocommit
       True if the statement is a "committable" statement
 
-    returns_rows
-      True if the statement should return result rows
-
     postfetch_cols
      a list of Column objects for which a server-side default
      or inline SQL expression value was fired off.  applies to inserts and updates.
@@ -835,7 +832,7 @@ class Connection(Connectable):
         context = self.__create_execution_context(statement=statement, parameters=parameters)
         self.__execute_raw(context)
         self._autocommit(context)
-        return context.result()
+        return context.get_result_proxy()
 
     def __distill_params(self, multiparams, params):
         """given arguments from the calling form *multiparams, **params, return a list
@@ -889,7 +886,7 @@ class Connection(Connectable):
         self.__execute_raw(context)
         context.post_execution()
         self._autocommit(context)
-        return context.result()
+        return context.get_result_proxy()
 
     def __execute_raw(self, context):
         if context.executemany:
@@ -1296,7 +1293,7 @@ class RowProxy(object):
 
         self.__parent = parent
         self.__row = row
-        if self.__parent._ResultProxy__echo:
+        if self.__parent._echo:
             self.__parent.context.engine.logger.debug("Row " + repr(row))
 
     def close(self):
@@ -1392,11 +1389,8 @@ class ResultProxy(object):
         self.closed = False
         self.cursor = context.cursor
         self.connection = context.root_connection
-        self.__echo = context.engine._should_log_info
-        if context.returns_rows:
-            self._init_metadata()
-        else:
-            self.close()
+        self._echo = context.engine._should_log_info
+        self._init_metadata()
     
     @property
     def rowcount(self):
@@ -1411,51 +1405,55 @@ class ResultProxy(object):
         return self.context.out_parameters
 
     def _init_metadata(self):
+        metadata = self.cursor.description
+        if metadata is None:
+            # no results, close
+            self.close()
+            return
+            
         self._props = util.PopulateDict(None)
         self._props.creator = self.__key_fallback()
         self.keys = []
-        metadata = self.cursor.description
 
-        if metadata is not None:
-            typemap = self.dialect.dbapi_type_map
+        typemap = self.dialect.dbapi_type_map
 
-            for i, item in enumerate(metadata):
-                colname = item[0].decode(self.dialect.encoding)
+        for i, item in enumerate(metadata):
+            colname = item[0].decode(self.dialect.encoding)
 
-                if '.' in colname:
-                    # sqlite will in some circumstances prepend table name to colnames, so strip
-                    origname = colname
-                    colname = colname.split('.')[-1]
-                else:
-                    origname = None
+            if '.' in colname:
+                # sqlite will in some circumstances prepend table name to colnames, so strip
+                origname = colname
+                colname = colname.split('.')[-1]
+            else:
+                origname = None
 
-                if self.context.result_map:
-                    try:
-                        (name, obj, type_) = self.context.result_map[colname.lower()]
-                    except KeyError:
-                        (name, obj, type_) = (colname, None, typemap.get(item[1], types.NULLTYPE))
-                else:
+            if self.context.result_map:
+                try:
+                    (name, obj, type_) = self.context.result_map[colname.lower()]
+                except KeyError:
                     (name, obj, type_) = (colname, None, typemap.get(item[1], types.NULLTYPE))
+            else:
+                (name, obj, type_) = (colname, None, typemap.get(item[1], types.NULLTYPE))
 
-                rec = (type_, type_.dialect_impl(self.dialect).result_processor(self.dialect), i)
+            rec = (type_, type_.dialect_impl(self.dialect).result_processor(self.dialect), i)
 
-                if self._props.setdefault(name.lower(), rec) is not rec:
-                    self._props[name.lower()] = (type_, self.__ambiguous_processor(name), 0)
+            if self._props.setdefault(name.lower(), rec) is not rec:
+                self._props[name.lower()] = (type_, self.__ambiguous_processor(name), 0)
 
-                # store the "origname" if we truncated (sqlite only)
-                if origname:
-                    if self._props.setdefault(origname.lower(), rec) is not rec:
-                        self._props[origname.lower()] = (type_, self.__ambiguous_processor(origname), 0)
+            # store the "origname" if we truncated (sqlite only)
+            if origname:
+                if self._props.setdefault(origname.lower(), rec) is not rec:
+                    self._props[origname.lower()] = (type_, self.__ambiguous_processor(origname), 0)
 
-                self.keys.append(colname)
-                self._props[i] = rec
-                if obj:
-                    for o in obj:
-                        self._props[o] = rec
+            self.keys.append(colname)
+            self._props[i] = rec
+            if obj:
+                for o in obj:
+                    self._props[o] = rec
 
-            if self.__echo:
-                self.context.engine.logger.debug(
-                    "Col " + repr(tuple(x[0] for x in metadata)))
+        if self._echo:
+            self.context.engine.logger.debug(
+                "Col " + repr(tuple(x[0] for x in metadata)))
     
     def __key_fallback(self):
         # create a closure without 'self' to avoid circular references
@@ -1481,18 +1479,24 @@ class ResultProxy(object):
 
     def __ambiguous_processor(self, colname):
         def process(value):
-            raise exc.InvalidRequestError("Ambiguous column name '%s' in result set! try 'use_labels' option on select statement." % colname)
+            raise exc.InvalidRequestError("Ambiguous column name '%s' in result set! "
+                        "try 'use_labels' option on select statement." % colname)
         return process
 
     def close(self):
-        """Close this ResultProxy, and the underlying DB-API cursor corresponding to the execution.
+        """Close this ResultProxy.
+        
+        Closes the underlying DBAPI cursor corresponding to the execution.
 
         If this ResultProxy was generated from an implicit execution,
         the underlying Connection will also be closed (returns the
-        underlying DB-API connection to the connection pool.)
+        underlying DBAPI connection to the connection pool.)
 
-        This method is also called automatically when all result rows
-        are exhausted.
+        This method is called automatically when:
+        
+            * all result rows are exhausted using the fetchXXX() methods.
+            * cursor.description is None.
+        
         """
         if not self.closed:
             self.closed = True
@@ -1663,7 +1667,9 @@ class BufferedRowResultProxy(ResultProxy):
     The pre-fetching behavior fetches only one row initially, and then
     grows its buffer size by a fixed amount with each successive need
     for additional rows up to a size of 100.
+    
     """
+
     def _init_metadata(self):
         self.__buffer_rows()
         super(BufferedRowResultProxy, self)._init_metadata()
@@ -1716,7 +1722,9 @@ class BufferedColumnResultProxy(ResultProxy):
     of scope unless explicitly fetched.  Currently this includes just
     cx_Oracle LOB objects, but this behavior is known to exist in
     other DB-APIs as well (Pygresql, currently unsupported).
+    
     """
+
     _process_row = BufferedColumnRow
 
     def _get_col(self, row, key):
@@ -1757,8 +1765,8 @@ class SchemaIterator(schema.SchemaVisitor):
     """A visitor that can gather text into a buffer and execute the contents of the buffer."""
 
     def __init__(self, connection):
-        """Construct a new SchemaIterator.
-        """
+        """Construct a new SchemaIterator."""
+        
         self.connection = connection
         self.buffer = StringIO.StringIO()
 
@@ -1783,6 +1791,7 @@ class DefaultRunner(schema.SchemaVisitor):
     DefaultRunners are used internally by Engines and Dialects.
     Specific database modules should provide their own subclasses of
     DefaultRunner to allow database-specific behavior.
+
     """
 
     def __init__(self, context):
@@ -1803,19 +1812,9 @@ class DefaultRunner(schema.SchemaVisitor):
             return None
 
     def visit_passive_default(self, default):
-        """Do nothing.
-
-        Passive defaults by definition return None on the app side,
-        and are post-fetched to get the DB-side value.
-        """
-
         return None
 
     def visit_sequence(self, seq):
-        """Do nothing.
-
-        """
-
         return None
 
     def exec_default_sql(self, default):
@@ -1824,8 +1823,8 @@ class DefaultRunner(schema.SchemaVisitor):
         return conn._execute_compiled(c).scalar()
 
     def execute_string(self, stmt, params=None):
-        """execute a string statement, using the raw cursor,
-        and return a scalar result."""
+        """execute a string statement, using the raw cursor, and return a scalar result."""
+        
         conn = self.context._connection
         if isinstance(stmt, unicode) and not self.dialect.supports_unicode_statements:
             stmt = stmt.encode(self.dialect.encoding)
index c176f7082dfdcbed5cc436dbc1cb38f0c201bf54..f99cac4659bb0c21f39160cfe737e529712960ed 100644 (file)
@@ -19,8 +19,6 @@ from sqlalchemy import exc
 
 AUTOCOMMIT_REGEXP = re.compile(r'\s*(?:UPDATE|INSERT|CREATE|DELETE|DROP|ALTER)',
                                re.I | re.UNICODE)
-SELECT_REGEXP = re.compile(r'\s*SELECT', re.I | re.UNICODE)
-
 
 class DefaultDialect(base.Dialect):
     """Default implementation of Dialect"""
@@ -152,10 +150,8 @@ class DefaultExecutionContext(base.ExecutionContext):
             self.isinsert = compiled.isinsert
             self.isupdate = compiled.isupdate
             if isinstance(compiled.statement, expression._TextClause):
-                self.returns_rows = self.returns_rows_text(self.statement)
                 self.should_autocommit = compiled.statement._autocommit or self.should_autocommit_text(self.statement)
             else:
-                self.returns_rows = self.returns_rows_compiled(compiled)
                 self.should_autocommit = getattr(compiled.statement, '_autocommit', False) or self.should_autocommit_compiled(compiled)
 
             if not parameters:
@@ -181,15 +177,16 @@ class DefaultExecutionContext(base.ExecutionContext):
                 self.statement = statement
             self.isinsert = self.isupdate = False
             self.cursor = self.create_cursor()
-            self.returns_rows = self.returns_rows_text(statement)
             self.should_autocommit = self.should_autocommit_text(statement)
         else:
             # no statement. used for standalone ColumnDefault execution.
             self.statement = None
-            self.isinsert = self.isupdate = self.executemany = self.returns_rows = self.should_autocommit = False
+            self.isinsert = self.isupdate = self.executemany = self.should_autocommit = False
             self.cursor = self.create_cursor()
 
-    connection = property(lambda s:s._connection._branch())
+    @property
+    def connection(self):
+        return self._connection._branch()
 
     def __encode_param_keys(self, params):
         """apply string encoding to the keys of dictionary-based bind parameters.
@@ -248,12 +245,6 @@ class DefaultExecutionContext(base.ExecutionContext):
                 parameters.append(param)
         return parameters
 
-    def returns_rows_compiled(self, compiled):
-        return isinstance(compiled.statement, expression.Selectable)
-
-    def returns_rows_text(self, statement):
-        return SELECT_REGEXP.match(statement)
-
     def should_autocommit_compiled(self, compiled):
         return isinstance(compiled.statement, expression._UpdateBase)
 
@@ -269,15 +260,12 @@ class DefaultExecutionContext(base.ExecutionContext):
     def post_execution(self):
         self.post_exec()
 
-    def result(self):
-        return self.get_result_proxy()
-
     def pre_exec(self):
         pass
 
     def post_exec(self):
         pass
-
+    
     def get_result_proxy(self):
         return base.ResultProxy(self)
 
index 41f6cdc5dfd7c53547113798619c24a29dae0673..eb21c141e761d79e8eacb0b17162bd3aff66c4c5 100644 (file)
@@ -340,7 +340,7 @@ class ZooMarkTest(TestBase):
     def test_profile_4_expressions(self):
         self.test_baseline_4_expressions()
 
-    @profiling.function_call_count(1523, {'2.4': 1084})
+    @profiling.function_call_count(1442, {'2.4': 1084})
     def test_profile_5_aggregates(self):
         self.test_baseline_5_aggregates()
 
index 54f96ddbfa707e114732cc9fedb09ee7fe369530..784bb35a28bc4a9031824a24b1695ecbd9472af8 100644 (file)
@@ -306,7 +306,7 @@ class ZooMarkTest(TestBase):
     def test_profile_4_expressions(self):
         self.test_baseline_4_expressions()
 
-    @profiling.function_call_count(1507)
+    @profiling.function_call_count(1426)
     def test_profile_5_aggregates(self):
         self.test_baseline_5_aggregates()