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 (file)
index 0000000..decc6a2
--- /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 992b97188a04dde56183aa3a6107156610a4d09b..507fcfdcb5b5679098a068c748648e251de64c44 100644 (file)
--- 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 9823faa814cc1c7ccd70dbb4f6479fa6fa690d27..26e6d152ccb192663557f078dd9c3e641895c556 100644 (file)
--- 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(