- The limit/offset keywords to select() as well

as the value passed to select.limit()/offset()
will be coerced to integer.  [ticket:2116]
(also in 0.6.7)
- Oracle dialect adds use_binds_for_limits=False
create_engine() flag, will render the LIMIT/OFFSET
values inline instead of as binds, reported to
modify the execution plan used by Oracle.
[ticket:2116] (Also in 0.6.7)

4 files changed, 48 insertions, 13 deletions

diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py
index 72411d735..14e6309cb 100644
--- a/lib/sqlalchemy/dialects/oracle/base.py
+++ b/lib/sqlalchemy/dialects/oracle/base.py
@@ -22,6 +22,8 @@ affect the behavior of the dialect regardless of driver in use.
 
 * *optimize_limits* - defaults to ``False``. see the section on LIMIT/OFFSET.
 
+* *use_binds_for_limits* - defaults to ``True``.  see the section on LIMIT/OFFSET.
+
 Auto Increment Behavior
 -----------------------
 
@@ -74,13 +76,27 @@ requires NLS_LANG to be set.
 LIMIT/OFFSET Support
 --------------------
 
-Oracle has no support for the LIMIT or OFFSET keywords.  Whereas previous versions of SQLAlchemy
-used the "ROW NUMBER OVER..." construct to simulate LIMIT/OFFSET, SQLAlchemy 0.5 now uses 
-a wrapped subquery approach in conjunction with ROWNUM.  The exact methodology is taken from
-http://www.oracle.com/technology/oramag/oracle/06-sep/o56asktom.html .  Note that the 
-"FIRST ROWS()" optimization keyword mentioned is not used by default, as the user community felt
-this was stepping into the bounds of optimization that is better left on the DBA side, but this
-prefix can be added by enabling the optimize_limits=True flag on create_engine().
+Oracle has no support for the LIMIT or OFFSET keywords.  SQLAlchemy uses 
+a wrapped subquery approach in conjunction with ROWNUM.  The exact methodology 
+is taken from
+http://www.oracle.com/technology/oramag/oracle/06-sep/o56asktom.html .  
+
+There are two options which affect its behavior:
+
+* the "FIRST ROWS()" optimization keyword is not used by default.  To enable the usage of this
+  optimization directive, specify ``optimize_limits=True`` to :func:`.create_engine`.
+* the values passed for the limit/offset are sent as bound parameters.   Some users have observed
+  that Oracle produces a poor query plan when the values are sent as binds and not
+  rendered literally.   To render the limit/offset values literally within the SQL 
+  statement, specify ``use_binds_for_limits=False`` to :func:`.create_engine`.
+
+Some users have reported better performance when the entirely different approach of a 
+window query is used, i.e. ROW_NUMBER() OVER (ORDER BY), to provide LIMIT/OFFSET (note 
+that the majority of users don't observe this).  To suit this case the 
+method used for LIMIT/OFFSET can be replaced entirely.  See the recipe at 
+http://www.sqlalchemy.org/trac/wiki/UsageRecipes/WindowFunctionsByDefault
+which installs a select compiler that overrides the generation of limit/offset with
+a window function.
 
 ON UPDATE CASCADE
 -----------------
@@ -524,6 +540,8 @@ class OracleCompiler(compiler.SQLCompiler):
                     max_row = select._limit
                     if select._offset is not None:
                         max_row += select._offset
+                    if not self.dialect.use_binds_for_limits:
+                        max_row = sql.literal_column("%d" % max_row)
                     limitselect.append_whereclause(
                             sql.literal_column("ROWNUM")<=max_row)
 
@@ -542,8 +560,11 @@ class OracleCompiler(compiler.SQLCompiler):
                     offsetselect._oracle_visit = True
                     offsetselect._is_wrapper = True
 
+                    offset_value = select._offset
+                    if not self.dialect.use_binds_for_limits:
+                        offset_value = sql.literal_column("%d" % offset_value)
                     offsetselect.append_whereclause(
-                             sql.literal_column("ora_rn")>select._offset)
+                             sql.literal_column("ora_rn")>offset_value)
 
                     offsetselect.for_update = select.for_update
                     select = offsetselect
@@ -635,10 +656,12 @@ class OracleDialect(default.DefaultDialect):
     def __init__(self, 
                 use_ansi=True, 
                 optimize_limits=False, 
+                use_binds_for_limits=True,
                 **kwargs):
         default.DefaultDialect.__init__(self, **kwargs)
         self.use_ansi = use_ansi
         self.optimize_limits = optimize_limits
+        self.use_binds_for_limits = use_binds_for_limits
 
     def initialize(self, connection):
         super(OracleDialect, self).initialize(connection)
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 3323dcca9..f5ec41a60 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3972,6 +3972,8 @@ class _SelectBase(Executable, FromClause):
 
     _order_by_clause = ClauseList()
     _group_by_clause = ClauseList()
+    _limit = None
+    _offset = None
 
     def __init__(self,
             use_labels=False,
@@ -3991,8 +3993,10 @@ class _SelectBase(Executable, FromClause):
             self._execution_options = \
                 self._execution_options.union({'autocommit'
                     : autocommit})
-        self._limit = limit
-        self._offset = offset
+        if limit is not None:
+            self._limit = util.asint(limit)
+        if offset is not None:
+            self._offset = util.asint(offset)
         self._bind = bind
 
         if order_by is not None:
@@ -4061,14 +4065,14 @@ class _SelectBase(Executable, FromClause):
         """return a new selectable with the given LIMIT criterion
         applied."""
 
-        self._limit = limit
+        self._limit = util.asint(limit)
 
     @_generative
     def offset(self, offset):
         """return a new selectable with the given OFFSET criterion
         applied."""
 
-        self._offset = offset
+        self._offset = util.asint(offset)
 
     @_generative
     def order_by(self, *clauses):
diff --git a/lib/sqlalchemy/util/__init__.py b/lib/sqlalchemy/util/__init__.py
index 7e8ce59c4..93c418eda 100644
--- a/lib/sqlalchemy/util/__init__.py
+++ b/lib/sqlalchemy/util/__init__.py
@@ -25,7 +25,7 @@ from langhelpers import iterate_attributes, class_hierarchy, \
     monkeypatch_proxied_specials, asbool, bool_or_str, coerce_kw_type,\
     duck_type_collection, assert_arg_type, symbol, dictlike_iteritems,\
     classproperty, set_creation_order, warn_exception, warn, NoneType,\
-    constructor_copy, methods_equivalent, chop_traceback
+    constructor_copy, methods_equivalent, chop_traceback, asint
 
 from deprecations import warn_deprecated, warn_pending_deprecation, \
     deprecated, pending_deprecation
diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py
index cbe0d2a4f..ba612bc2c 100644
--- a/lib/sqlalchemy/util/langhelpers.py
+++ b/lib/sqlalchemy/util/langhelpers.py
@@ -551,6 +551,14 @@ def bool_or_str(*text):
             return asbool(obj)
     return bool_or_value
 
+def asint(value):
+    """Coerce to integer."""
+
+    if value is None:
+        return value
+    return int(value)
+
+
 def coerce_kw_type(kw, key, type_, flexi_bool=True):
     """If 'key' is present in dict 'kw', coerce its value to type 'type\_' if
     necessary.  If 'flexi_bool' is True, the string '0' is considered false