Fix label referencing in SQL Server OFFSET logic

Fixed bug where by ROW_NUMBER OVER clause applied for OFFSET
selects in SQL Server would inappropriately substitute a plain column
from the local statement that overlaps with a label name used by
the ORDER BY criteria of the statement.

Change-Id: Ic2500c886cbfc83a1ad5a2681783f008b9f23838
Fixes: #3711
(cherry picked from commit a4be7c92393e08607dc46f318e97803519052a93)

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index c51040dd5569e56a211c774365f4208a8aabcba8..e508e7b8105e0cc0267c4318a99427c14ef6b564 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -18,6 +18,15 @@
 .. changelog::
     :version: 1.0.13
 
+    .. change::
+        :tags: bug, mssql
+        :tickets: 3711
+
+        Fixed bug where by ROW_NUMBER OVER clause applied for OFFSET
+        selects in SQL Server would inappropriately substitute a plain column
+        from the local statement that overlaps with a label name used by
+        the ORDER BY criteria of the statement.
+
     .. change::
         :tags: bug, orm
         :tickets: 3710

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index f5972632f7af95be4af060a7596fe1a12bee277a..927dceff74af55d26fc36f6ff9183786943a5663 100644 (file)
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -1129,7 +1129,11 @@ class MSSQLCompiler(compiler.SQLCompiler):
                                        'using an OFFSET or a non-simple '
                                        'LIMIT clause')
 
-            _order_by_clauses = select._order_by_clause.clauses
+            _order_by_clauses = [
+                sql_util.unwrap_label_reference(elem)
+                for elem in select._order_by_clause.clauses
+            ]
+
             limit_clause = select._limit_clause
             offset_clause = select._offset_clause
             kwargs['select_wraps_for'] = select

diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 16a1421f6dfc37b7b00d594b96379c9c41f6b126..665814f8453d715ba9d0f2b40dfb9e5e75fe9e3e 100644 (file)
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -173,6 +173,16 @@ def unwrap_order_by(clause):
     return cols
 
 
+def unwrap_label_reference(element):
+    def replace(elem):
+        if isinstance(elem, (_label_reference, _textual_label_reference)):
+            return elem.element
+
+    return visitors.replacement_traverse(
+        element, {}, replace
+    )
+
+
 def clause_is_present(clause, search):
     """Given a target clause and a second to search within, return True
     if the target is plainly present in the search without any

diff --git a/test/dialect/mssql/test_compiler.py b/test/dialect/mssql/test_compiler.py
index ee36bdca788f9ed52e14e5cec27fbc5d5a5843ee..1f3c0ba321a401bb69709251f62e53ded9bedae8 100644 (file)
--- a/test/dialect/mssql/test_compiler.py
+++ b/test/dialect/mssql/test_compiler.py
@@ -571,6 +571,31 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL):
         assert t1.c.x in set(c._create_result_map()['x'][1])
         assert t1.c.y in set(c._create_result_map()['y'][1])
 
+    def test_offset_dont_misapply_labelreference(self):
+        m = MetaData()
+
+        t = Table('t', m, Column('x', Integer))
+
+        expr1 = func.foo(t.c.x).label('x')
+        expr2 = func.foo(t.c.x).label('y')
+
+        stmt1 = select([expr1]).order_by(expr1.desc()).offset(1)
+        stmt2 = select([expr2]).order_by(expr2.desc()).offset(1)
+
+        self.assert_compile(
+            stmt1,
+            "SELECT anon_1.x FROM (SELECT foo(t.x) AS x, "
+            "ROW_NUMBER() OVER (ORDER BY foo(t.x) DESC) AS mssql_rn FROM t) "
+            "AS anon_1 WHERE mssql_rn > :param_1"
+        )
+
+        self.assert_compile(
+            stmt2,
+            "SELECT anon_1.y FROM (SELECT foo(t.x) AS y, "
+            "ROW_NUMBER() OVER (ORDER BY foo(t.x) DESC) AS mssql_rn FROM t) "
+            "AS anon_1 WHERE mssql_rn > :param_1"
+        )
+
     def test_limit_zero_offset_using_window(self):
         t = table('t', column('x', Integer), column('y', Integer))