- FOR UPDATE is emitted in the syntactically correct position

when limit/offset is used, i.e. the ROWNUM subquery.
However, Oracle can't really handle FOR UPDATE with ORDER BY
or with subqueries, so its still not very usable, but at
least SQLA gets the SQL past the Oracle parser.
[ticket:1815]

diff --git a/CHANGES b/CHANGES
index 997e046c10445a17212d85df52b15d0483360655..2c9aef21842764c65b16278455f36cd9576e89e2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -85,6 +85,13 @@ CHANGES
     in connect strings here since we don't know what encoding
     we could use.  [ticket:1670]
 
+  - FOR UPDATE is emitted in the syntactically correct position
+    when limit/offset is used, i.e. the ROWNUM subquery.
+    However, Oracle can't really handle FOR UPDATE with ORDER BY
+    or with subqueries, so its still not very usable, but at
+    least SQLA gets the SQL past the Oracle parser.
+    [ticket:1815]
+    
 - firebird
   - Added a label to the query used within has_table() and 
     has_sequence() to work with older versions of Firebird

diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py
index 6c8055138ac33642d70669634c575f531a9ba884..cd232fa007cb54408ea25942dac3199b7ac68dd9 100644 (file)
--- a/lib/sqlalchemy/dialects/oracle/base.py
+++ b/lib/sqlalchemy/dialects/oracle/base.py
@@ -479,7 +479,7 @@ class OracleCompiler(compiler.SQLCompiler):
 
                 limitselect._oracle_visit = True
                 limitselect._is_wrapper = True
-
+                
                 # If needed, add the limiting clause
                 if select._limit is not None:
                     max_row = select._limit
@@ -490,22 +490,24 @@ class OracleCompiler(compiler.SQLCompiler):
 
                 # If needed, add the ora_rn, and wrap again with offset.
                 if select._offset is None:
+                    limitselect.for_update = select.for_update
                     select = limitselect
                 else:
-                     limitselect = limitselect.column(
+                    limitselect = limitselect.column(
                              sql.literal_column("ROWNUM").label("ora_rn"))
-                     limitselect._oracle_visit = True
-                     limitselect._is_wrapper = True
+                    limitselect._oracle_visit = True
+                    limitselect._is_wrapper = True
 
-                     offsetselect = sql.select(
+                    offsetselect = sql.select(
                              [c for c in limitselect.c if c.key!='ora_rn'])
-                     offsetselect._oracle_visit = True
-                     offsetselect._is_wrapper = True
+                    offsetselect._oracle_visit = True
+                    offsetselect._is_wrapper = True
 
-                     offsetselect.append_whereclause(
+                    offsetselect.append_whereclause(
                              sql.literal_column("ora_rn")>select._offset)
 
-                     select = offsetselect
+                    offsetselect.for_update = select.for_update
+                    select = offsetselect
 
         kwargs['iswrapper'] = getattr(select, '_is_wrapper', False)
         return compiler.SQLCompiler.visit_select(self, select, **kwargs)
@@ -514,7 +516,9 @@ class OracleCompiler(compiler.SQLCompiler):
         return ""
 
     def for_update_clause(self, select):
-        if select.for_update == "nowait":
+        if self.is_subquery():
+            return ""
+        elif select.for_update == "nowait":
             return " FOR UPDATE NOWAIT"
         else:
             return super(OracleCompiler, self).for_update_clause(select)

diff --git a/test/dialect/test_oracle.py b/test/dialect/test_oracle.py
index b9fb9a1337964bb33f2befcb86133b8288aa9864..9508b38bf4cf172ed1834fcb4d53c5d0a7d35f43 100644 (file)
--- a/test/dialect/test_oracle.py
+++ b/test/dialect/test_oracle.py
@@ -5,7 +5,7 @@ from sqlalchemy import *
 from sqlalchemy import types as sqltypes, exc
 from sqlalchemy.sql import table, column
 from sqlalchemy.test import *
-from sqlalchemy.test.testing import eq_, assert_raises
+from sqlalchemy.test.testing import eq_, assert_raises, assert_raises_message
 from sqlalchemy.test.engines import testing_engine
 from sqlalchemy.dialects.oracle import cx_oracle, base as oracle
 from sqlalchemy.engine import default
@@ -113,6 +113,25 @@ class CompileTest(TestBase, AssertsCompiledSQL):
         self.assert_compile(s, "SELECT col1, col2 FROM (SELECT col1, col2, ROWNUM "
             "AS ora_rn FROM (SELECT sometable.col1 AS col1, sometable.col2 AS col2 FROM sometable "
             "ORDER BY sometable.col2) WHERE ROWNUM <= :ROWNUM_1) WHERE ora_rn > :ora_rn_1")
+
+        s = select([t], for_update=True).limit(10).order_by(t.c.col2)
+        self.assert_compile(
+            s,
+            "SELECT col1, col2 FROM (SELECT sometable.col1 "
+            "AS col1, sometable.col2 AS col2 FROM sometable "
+            "ORDER BY sometable.col2) WHERE ROWNUM <= :ROWNUM_1 FOR UPDATE"
+        )
+        
+        s = select([t], for_update=True).limit(10).offset(20).order_by(t.c.col2)
+        self.assert_compile(
+            s,
+            "SELECT col1, col2 FROM (SELECT col1, col2, ROWNUM "
+            "AS ora_rn FROM (SELECT sometable.col1 AS col1, "
+            "sometable.col2 AS col2 FROM sometable ORDER BY "
+            "sometable.col2) WHERE ROWNUM <= :ROWNUM_1) WHERE "
+            "ora_rn > :ora_rn_1 FOR UPDATE"
+        )
+        
     
     def test_long_labels(self):
         dialect = default.DefaultDialect()
@@ -924,9 +943,45 @@ class SequenceTest(TestBase, AssertsCompiledSQL):
 
 class ExecuteTest(TestBase):
     __only_on__ = 'oracle'
+    
+    
     def test_basic(self):
         eq_(
             testing.db.execute("/*+ this is a comment */ SELECT 1 FROM DUAL").fetchall(),
             [(1,)]
         )
 
+    @testing.provide_metadata
+    def test_limit_offset_for_update(self):
+        # oracle can't actually do the ROWNUM thing with FOR UPDATE
+        # very well.
+        
+        t = Table('t1', metadata, Column('id', Integer, primary_key=True),
+            Column('data', Integer)
+        )
+        metadata.create_all()
+        
+        t.insert().execute(
+            {'id':1, 'data':1},
+            {'id':2, 'data':7},
+            {'id':3, 'data':12},
+            {'id':4, 'data':15},
+            {'id':5, 'data':32},
+        )
+        
+        # here, we can't use ORDER BY.
+        eq_(
+            t.select(for_update=True).limit(2).execute().fetchall(),
+            [(1, 1),
+             (2, 7)]
+        )
+
+        # here, its impossible.  But we'd prefer it to raise ORA-02014
+        # instead of issuing a syntax error.
+        assert_raises_message(
+            exc.DatabaseError,
+            "ORA-02014",
+            t.select(for_update=True).limit(2).offset(3).execute
+        )
+        
+