From: Matt Lewellyn <guruofgentoo@gmail.com>
Date: Wed, 3 Apr 2019 22:39:15 +0000 (-0400)
Subject: MSSQL: only compile ORDER BY if it will be rendered
X-Git-Tag: rel_1_3_3~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=97d4d15fde7999eba29c9708b65e11d82623f686;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

MSSQL: only compile ORDER BY if it will be rendered

Fixed issue in SQL Server dialect where if a bound parameter were present in
an ORDER BY expression that would ultimately not be rendered in the SQL
Server version of the statement, the parameters would still be part of the
execution parameters, leading to DBAPI-level errors.  Pull request courtesy
Matt Lewellyn.

Fixes: #4587
Closes: #4588
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/4588
Pull-request-sha: 2992a473e0f6d4fc27794cfd949ba20a81fad2ca

Change-Id: Ie709aefdb1babf810bb81526289448f8cc7a4cb1
---

diff --git a/doc/build/changelog/unreleased_13/4587.rst b/doc/build/changelog/unreleased_13/4587.rst
new file mode 100644
index 0000000000..decc6a286b
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4587.rst
@@ -0,0 +1,9 @@
+.. change::
+   :tags: bug, mssql
+   :tickets: 4587
+
+   Fixed issue in SQL Server dialect where if a bound parameter were present in
+   an ORDER BY expression that would ultimately not be rendered in the SQL
+   Server version of the statement, the parameters would still be part of the
+   execution parameters, leading to DBAPI-level errors.  Pull request courtesy
+   Matt Lewellyn.
diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index 992b97188a..507fcfdcb5 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -1753,10 +1753,16 @@ class MSSQLCompiler(compiler.SQLCompiler):
         return ""
 
     def order_by_clause(self, select, **kw):
+        # MSSQL only allows ORDER BY in subqueries if there is a LIMIT
+        if self.is_subquery() and not select._limit:
+            # avoid processing the order by clause if we won't end up
+            # using it, because we don't want all the bind params tacked
+            # onto the positional list if that is what the dbapi requires
+            return ""
+
         order_by = self.process(select._order_by_clause, **kw)
 
-        # MSSQL only allows ORDER BY in subqueries if there is a LIMIT
-        if order_by and (not self.is_subquery() or select._limit):
+        if order_by:
             return " ORDER BY " + order_by
         else:
             return ""
diff --git a/test/dialect/mssql/test_compiler.py b/test/dialect/mssql/test_compiler.py
index 9823faa814..26e6d152cc 100644
--- a/test/dialect/mssql/test_compiler.py
+++ b/test/dialect/mssql/test_compiler.py
@@ -377,6 +377,36 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL):
             "foo.myid = mytable.myid",
         )
 
+    def test_noorderby_parameters_insubquery(self):
+        """test that the ms-sql dialect does not include ORDER BY
+        positional parameters in subqueries"""
+
+        table1 = table(
+            "mytable",
+            column("myid", Integer),
+            column("name", String),
+            column("description", String),
+        )
+
+        q = select(
+            [table1.c.myid, sql.literal('bar').label('c1')],
+            order_by=[table1.c.name + '-']
+        ).alias("foo")
+        crit = q.c.myid == table1.c.myid
+        dialect = mssql.dialect()
+        dialect.paramstyle = "qmark"
+        dialect.positional = True
+        self.assert_compile(
+            select(["*"], crit),
+            "SELECT * FROM (SELECT mytable.myid AS "
+            "myid, ? AS c1 FROM mytable) AS foo, mytable WHERE "
+            "foo.myid = mytable.myid",
+            dialect=dialect,
+            checkparams={'param_1': 'bar'},
+            # if name_1 is included, too many parameters are passed to dbapi
+            checkpositional=('bar', )
+        )
+
     def test_force_schema_quoted_name_w_dot_case_insensitive(self):
         metadata = MetaData()
         tbl = Table(