Merge branch 'master' into ticket_3100

15 files changed, 150 insertions, 95 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 643035477..e9b78fe78 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -22,6 +22,20 @@
     on compatibility concerns, see :doc:`/changelog/migration_10`.
 
     .. change::
+        :tags: changed, mssql
+        :tickets: 3182
+
+        The hostname-based connection format for SQL Server when using
+        pyodbc will no longer specify a default "driver name", and a warning
+        is emitted if this is missing.  The optimal driver name for SQL Server
+        changes frequently and is per-platform, so hostname based connections
+        need to specify this.  DSN-based connections are preferred.
+
+        .. seealso::
+
+            :ref:`change_3182`
+
+    .. change::
         :tags: changed, sql
 
         The :func:`~.expression.column` and :func:`~.expression.table`
@@ -361,7 +375,11 @@
 		here is fully backwards compatible with existing third party dialects,
 		however those dialects which implement special LIMIT/OFFSET systems
 		will need modification in order to take advantage of the new
-		capabilities.  Work on this feature is courtesy of Dobes Vandermeer.
+		capabilities.  Limit and offset also support "literal_binds" mode,
+        where bound parameters are rendered inline as strings based on
+        a compile-time option.
+        Work on this feature is courtesy of Dobes Vandermeer.
+
 
 		.. seealso::
 
diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst
index 8acaa0445..58aa42df0 100644
--- a/doc/build/changelog/migration_10.rst
+++ b/doc/build/changelog/migration_10.rst
@@ -808,6 +808,24 @@ a cursor to be closed unless all results are fully fetched.
 
 :ticket:`2515`
 
+.. _change_3182:
+
+PyODBC driver name is required with hostname-based SQL Server connections
+-------------------------------------------------------------------------
+
+Connecting to SQL Server with PyODBC using a DSN-less connection, e.g.
+with an explicit hostname, now requires a driver name - SQLAlchemy will no
+longer attempt to guess a default::
+
+    engine = create_engine("mssql+pyodbc://scott:tiger@myhost:port/databasename?driver=SQL+Server+Native+Client+10.0")
+
+SQLAlchemy's previously hardcoded default of "SQL Server" is obsolete on
+Windows, and SQLAlchemy cannot be tasked with guessing the best driver
+based on operation system/driver detection.   Using a DSN is always preferred
+when using ODBC to avoid this issue entirely.
+
+:ticket:`3182`
+
 .. _change_2984:
 
 Drizzle Dialect is now an External Dialect
diff --git a/examples/performance/large_resultsets.py b/examples/performance/large_resultsets.py
index 268c6dc87..7383db734 100644
--- a/examples/performance/large_resultsets.py
+++ b/examples/performance/large_resultsets.py
@@ -132,8 +132,6 @@ def _test_dbapi_raw(n, fetchall):
 
     sql = str(compiled)
 
-    import pdb
-    pdb.set_trace()
     conn = engine.raw_connection()
     cursor = conn.cursor()
     cursor.execute(sql)
diff --git a/lib/sqlalchemy/connectors/pyodbc.py b/lib/sqlalchemy/connectors/pyodbc.py
index ef72c8049..907e4d353 100644
--- a/lib/sqlalchemy/connectors/pyodbc.py
+++ b/lib/sqlalchemy/connectors/pyodbc.py
@@ -26,7 +26,7 @@ class PyODBCConnector(Connector):
     supports_native_decimal = True
     default_paramstyle = 'named'
 
-    # for non-DSN connections, this should
+    # for non-DSN connections, this *may* be used to
     # hold the desired driver name
     pyodbc_driver_name = None
 
@@ -75,10 +75,21 @@ class PyODBCConnector(Connector):
                 if 'port' in keys and 'port' not in query:
                     port = ',%d' % int(keys.pop('port'))
 
-                connectors = ["DRIVER={%s}" %
-                              keys.pop('driver', self.pyodbc_driver_name),
-                              'Server=%s%s' % (keys.pop('host', ''), port),
-                              'Database=%s' % keys.pop('database', '')]
+                connectors = []
+                driver = keys.pop('driver', self.pyodbc_driver_name)
+                if driver is None:
+                    util.warn(
+                        "No driver name specified; "
+                        "this is expected by PyODBC when using "
+                        "DSN-less connections")
+                else:
+                    connectors.append("DRIVER={%s}" % driver)
+
+                connectors.extend(
+                    [
+                        'Server=%s%s' % (keys.pop('host', ''), port),
+                        'Database=%s' % keys.pop('database', '')
+                    ])
 
             user = keys.pop("user", None)
             if user:
diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index d1d4cb9ca..ba3050ae5 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -924,7 +924,7 @@ class MSSQLCompiler(compiler.SQLCompiler):
     def get_crud_hint_text(self, table, text):
         return text
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         # Limit in mssql is after the select keyword
         return ""
 
diff --git a/lib/sqlalchemy/dialects/mssql/pyodbc.py b/lib/sqlalchemy/dialects/mssql/pyodbc.py
index 1c75fe1ff..445584d24 100644
--- a/lib/sqlalchemy/dialects/mssql/pyodbc.py
+++ b/lib/sqlalchemy/dialects/mssql/pyodbc.py
@@ -12,74 +12,57 @@
     :connectstring: mssql+pyodbc://<username>:<password>@<dsnname>
     :url: http://pypi.python.org/pypi/pyodbc/
 
-Additional Connection Examples
--------------------------------
+Connecting to PyODBC
+--------------------
 
-Examples of pyodbc connection string URLs:
+The URL here is to be translated to PyODBC connection strings, as
+detailed in `ConnectionStrings <https://code.google.com/p/pyodbc/wiki/ConnectionStrings>`_.
 
-* ``mssql+pyodbc://mydsn`` - connects using the specified DSN named ``mydsn``.
-  The connection string that is created will appear like::
+DSN Connections
+^^^^^^^^^^^^^^^
 
-    dsn=mydsn;Trusted_Connection=Yes
+A DSN-based connection is **preferred** overall when using ODBC.  A
+basic DSN-based connection looks like::
 
-* ``mssql+pyodbc://user:pass@mydsn`` - connects using the DSN named
-  ``mydsn`` passing in the ``UID`` and ``PWD`` information. The
-  connection string that is created will appear like::
+    engine = create_engine("mssql+pyodbc://scott:tiger@some_dsn")
 
-    dsn=mydsn;UID=user;PWD=pass
-
-* ``mssql+pyodbc://user:pass@mydsn/?LANGUAGE=us_english`` - connects
-  using the DSN named ``mydsn`` passing in the ``UID`` and ``PWD``
-  information, plus the additional connection configuration option
-  ``LANGUAGE``. The connection string that is created will appear
-  like::
-
-    dsn=mydsn;UID=user;PWD=pass;LANGUAGE=us_english
-
-* ``mssql+pyodbc://user:pass@host/db`` - connects using a connection
-  that would appear like::
+Which above, will pass the following connection string to PyODBC::
 
-    DRIVER={SQL Server};Server=host;Database=db;UID=user;PWD=pass
-
-* ``mssql+pyodbc://user:pass@host:123/db`` - connects using a connection
-  string which includes the port
-  information using the comma syntax. This will create the following
-  connection string::
-
-    DRIVER={SQL Server};Server=host,123;Database=db;UID=user;PWD=pass
+    dsn=mydsn;UID=user;PWD=pass
 
-* ``mssql+pyodbc://user:pass@host/db?port=123`` - connects using a connection
-  string that includes the port
-  information as a separate ``port`` keyword. This will create the
-  following connection string::
+If the username and password are omitted, the DSN form will also add
+the ``Trusted_Connection=yes`` directive to the ODBC string.
 
-    DRIVER={SQL Server};Server=host;Database=db;UID=user;PWD=pass;port=123
+Hostname Connections
+^^^^^^^^^^^^^^^^^^^^
 
-* ``mssql+pyodbc://user:pass@host/db?driver=MyDriver`` - connects using a
-  connection string that includes a custom ODBC driver name.  This will create
-  the following connection string::
+Hostname-based connections are **not preferred**, however are supported.
+The ODBC driver name must be explicitly specified::
 
-    DRIVER={MyDriver};Server=host;Database=db;UID=user;PWD=pass
+    engine = create_engine("mssql+pyodbc://scott:tiger@myhost:port/databasename?driver=SQL+Server+Native+Client+10.0")
 
-If you require a connection string that is outside the options
-presented above, use the ``odbc_connect`` keyword to pass in a
-urlencoded connection string. What gets passed in will be urldecoded
-and passed directly.
+.. versionchanged:: 1.0.0 Hostname-based PyODBC connections now require the
+   SQL Server driver name specified explicitly.  SQLAlchemy cannot
+   choose an optimal default here as it varies based on platform
+   and installed drivers.
 
-For example::
+Other keywords interpreted by the Pyodbc dialect to be passed to
+``pyodbc.connect()`` in both the DSN and hostname cases include:
+``odbc_autotranslate``, ``ansi``, ``unicode_results``, ``autocommit``.
 
-    mssql+pyodbc:///?odbc_connect=dsn%3Dmydsn%3BDatabase%3Ddb
+Pass through exact Pyodbc string
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-would create the following connection string::
+A PyODBC connection string can also be sent exactly as specified in
+`ConnectionStrings <https://code.google.com/p/pyodbc/wiki/ConnectionStrings>`_
+into the driver using the parameter ``odbc_connect``.  The delimeters must be URL escaped, however,
+as illustrated below using ``urllib.quote_plus``::
 
-    dsn=mydsn;Database=db
+    import urllib
+    params = urllib.quote_plus("DRIVER={SQL Server Native Client 10.0};SERVER=dagger;DATABASE=test;UID=user;PWD=password")
 
-Encoding your connection string can be easily accomplished through
-the python shell. For example::
+    engine = create_engine("mssql+pyodbc:///?odbc_connect=%s" % params)
 
-    >>> import urllib
-    >>> urllib.quote_plus('dsn=mydsn;Database=db')
-    'dsn%3Dmydsn%3BDatabase%3Ddb'
 
 Unicode Binds
 -------------
@@ -243,8 +226,6 @@ class MSDialect_pyodbc(PyODBCConnector, MSDialect):
 
     execution_ctx_cls = MSExecutionContext_pyodbc
 
-    pyodbc_driver_name = 'SQL Server'
-
     colspecs = util.update_copy(
         MSDialect.colspecs,
         {
diff --git a/lib/sqlalchemy/dialects/mysql/base.py b/lib/sqlalchemy/dialects/mysql/base.py
index 012d178e7..4dccd2760 100644
--- a/lib/sqlalchemy/dialects/mysql/base.py
+++ b/lib/sqlalchemy/dialects/mysql/base.py
@@ -1662,13 +1662,13 @@ class MySQLCompiler(compiler.SQLCompiler):
              " ON ",
              self.process(join.onclause, **kwargs)))
 
-    def for_update_clause(self, select):
+    def for_update_clause(self, select, **kw):
         if select._for_update_arg.read:
             return " LOCK IN SHARE MODE"
         else:
             return " FOR UPDATE"
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         # MySQL supports:
         #   LIMIT <limit>
         #   LIMIT <offset>, <limit>
@@ -1694,15 +1694,15 @@ class MySQLCompiler(compiler.SQLCompiler):
                 # bound as part of MySQL's "syntax" for OFFSET with
                 # no LIMIT
                 return ' \n LIMIT %s, %s' % (
-                    self.process(offset_clause),
+                    self.process(offset_clause, **kw),
                     "18446744073709551615")
             else:
                 return ' \n LIMIT %s, %s' % (
-                    self.process(offset_clause),
-                    self.process(limit_clause))
+                    self.process(offset_clause, **kw),
+                    self.process(limit_clause, **kw))
         else:
             # No offset provided, so just use the limit
-            return ' \n LIMIT %s' % (self.process(limit_clause),)
+            return ' \n LIMIT %s' % (self.process(limit_clause, **kw),)
 
     def update_limit_clause(self, update_stmt):
         limit = update_stmt.kwargs.get('%s_limit' % self.dialect.name, None)
diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py
index 40ba051f7..81a9f1a95 100644
--- a/lib/sqlalchemy/dialects/oracle/base.py
+++ b/lib/sqlalchemy/dialects/oracle/base.py
@@ -740,10 +740,10 @@ class OracleCompiler(compiler.SQLCompiler):
         kwargs['iswrapper'] = getattr(select, '_is_wrapper', False)
         return compiler.SQLCompiler.visit_select(self, select, **kwargs)
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         return ""
 
-    def for_update_clause(self, select):
+    def for_update_clause(self, select, **kw):
         if self.is_subquery():
             return ""
 
@@ -751,7 +751,7 @@ class OracleCompiler(compiler.SQLCompiler):
 
         if select._for_update_arg.of:
             tmp += ' OF ' + ', '.join(
-                self.process(elem) for elem in
+                self.process(elem, **kw) for elem in
                 select._for_update_arg.of
             )
 
diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py
index f1418f903..575d2a6dd 100644
--- a/lib/sqlalchemy/dialects/postgresql/base.py
+++ b/lib/sqlalchemy/dialects/postgresql/base.py
@@ -1309,14 +1309,14 @@ class PGCompiler(compiler.SQLCompiler):
     def visit_sequence(self, seq):
         return "nextval('%s')" % self.preparer.format_sequence(seq)
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         text = ""
         if select._limit_clause is not None:
-            text += " \n LIMIT " + self.process(select._limit_clause)
+            text += " \n LIMIT " + self.process(select._limit_clause, **kw)
         if select._offset_clause is not None:
             if select._limit_clause is None:
                 text += " \n LIMIT ALL"
-            text += " OFFSET " + self.process(select._offset_clause)
+            text += " OFFSET " + self.process(select._offset_clause, **kw)
         return text
 
     def format_from_hint_text(self, sqltext, table, hint, iscrud):
@@ -1337,7 +1337,7 @@ class PGCompiler(compiler.SQLCompiler):
         else:
             return ""
 
-    def for_update_clause(self, select):
+    def for_update_clause(self, select, **kw):
 
         if select._for_update_arg.read:
             tmp = " FOR SHARE"
@@ -1349,7 +1349,7 @@ class PGCompiler(compiler.SQLCompiler):
                 c.table if isinstance(c, expression.ColumnClause)
                 else c for c in select._for_update_arg.of)
             tmp += " OF " + ", ".join(
-                self.process(table, ashint=True)
+                self.process(table, ashint=True, **kw)
                 for table in tables
             )
 
diff --git a/lib/sqlalchemy/dialects/sqlite/base.py b/lib/sqlalchemy/dialects/sqlite/base.py
index 3c8b2d4f7..af793d275 100644
--- a/lib/sqlalchemy/dialects/sqlite/base.py
+++ b/lib/sqlalchemy/dialects/sqlite/base.py
@@ -591,19 +591,19 @@ class SQLiteCompiler(compiler.SQLCompiler):
             raise exc.CompileError(
                 "%s is not a valid extract argument." % extract.field)
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         text = ""
         if select._limit_clause is not None:
-            text += "\n LIMIT " + self.process(select._limit_clause)
+            text += "\n LIMIT " + self.process(select._limit_clause, **kw)
         if select._offset_clause is not None:
             if select._limit_clause is None:
                 text += "\n LIMIT " + self.process(sql.literal(-1))
-            text += " OFFSET " + self.process(select._offset_clause)
+            text += " OFFSET " + self.process(select._offset_clause, **kw)
         else:
-            text += " OFFSET " + self.process(sql.literal(0))
+            text += " OFFSET " + self.process(sql.literal(0), **kw)
         return text
 
-    def for_update_clause(self, select):
+    def for_update_clause(self, select, **kw):
         # sqlite has no "FOR UPDATE" AFAICT
         return ''
 
diff --git a/lib/sqlalchemy/dialects/sybase/base.py b/lib/sqlalchemy/dialects/sybase/base.py
index 26f5ef04a..f65a76a27 100644
--- a/lib/sqlalchemy/dialects/sybase/base.py
+++ b/lib/sqlalchemy/dialects/sybase/base.py
@@ -346,7 +346,7 @@ class SybaseSQLCompiler(compiler.SQLCompiler):
     def get_from_hint_text(self, table, text):
         return text
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         # Limit in sybase is after the select keyword
         return ""
 
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 23e5456a7..e92520620 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1593,10 +1593,10 @@ class SQLCompiler(Compiled):
 
         if (select._limit_clause is not None or
                 select._offset_clause is not None):
-            text += self.limit_clause(select)
+            text += self.limit_clause(select, **kwargs)
 
         if select._for_update_arg is not None:
-            text += self.for_update_clause(select)
+            text += self.for_update_clause(select, **kwargs)
 
         if self.ctes and \
                 compound_index == 0 and toplevel:
@@ -1653,7 +1653,7 @@ class SQLCompiler(Compiled):
         else:
             return ""
 
-    def for_update_clause(self, select):
+    def for_update_clause(self, select, **kw):
         return " FOR UPDATE"
 
     def returning_clause(self, stmt, returning_cols):
@@ -1661,14 +1661,14 @@ class SQLCompiler(Compiled):
             "RETURNING is not supported by this "
             "dialect's statement compiler.")
 
-    def limit_clause(self, select):
+    def limit_clause(self, select, **kw):
         text = ""
         if select._limit_clause is not None:
-            text += "\n LIMIT " + self.process(select._limit_clause)
+            text += "\n LIMIT " + self.process(select._limit_clause, **kw)
         if select._offset_clause is not None:
             if select._limit_clause is None:
                 text += "\n LIMIT -1"
-            text += " OFFSET " + self.process(select._offset_clause)
+            text += " OFFSET " + self.process(select._offset_clause, **kw)
         return text
 
     def visit_table(self, table, asfrom=False, iscrud=False, ashint=False,
diff --git a/lib/sqlalchemy/testing/suite/test_select.py b/lib/sqlalchemy/testing/suite/test_select.py
index 3f14ada05..68dadd0a9 100644
--- a/lib/sqlalchemy/testing/suite/test_select.py
+++ b/lib/sqlalchemy/testing/suite/test_select.py
@@ -136,6 +136,21 @@ class LimitOffsetTest(fixtures.TablesTest):
             [(2, 2, 3), (3, 3, 4)]
         )
 
+    def test_limit_offset_nobinds(self):
+        """test that 'literal binds' mode works - no bound params."""
+
+        table = self.tables.some_table
+        stmt = select([table]).order_by(table.c.id).limit(2).offset(1)
+        sql = stmt.compile(
+            dialect=config.db.dialect,
+            compile_kwargs={"literal_binds": True})
+        sql = str(sql)
+
+        self._assert_result(
+            sql,
+            [(2, 2, 3), (3, 3, 4)]
+        )
+
     @testing.requires.bound_limit_offset
     def test_bound_limit(self):
         table = self.tables.some_table
diff --git a/test/dialect/mssql/test_engine.py b/test/dialect/mssql/test_engine.py
index c07f30040..8ac9c6c16 100644
--- a/test/dialect/mssql/test_engine.py
+++ b/test/dialect/mssql/test_engine.py
@@ -6,7 +6,7 @@ from sqlalchemy.dialects.mssql import pyodbc, pymssql
 from sqlalchemy.engine import url
 from sqlalchemy.testing import fixtures
 from sqlalchemy import testing
-from sqlalchemy.testing import assert_raises_message
+from sqlalchemy.testing import assert_raises_message, assert_warnings
 
 class ParseConnectTest(fixtures.TestBase):
 
@@ -38,18 +38,32 @@ class ParseConnectTest(fixtures.TestBase):
         assert ";LANGUAGE=us_english" in dsn_string
         assert ";foo=bar" in dsn_string
 
-    def test_pyodbc_connect(self):
+    def test_pyodbc_hostname(self):
         dialect = pyodbc.dialect()
-        u = url.make_url('mssql://username:password@hostspec/database')
+        u = url.make_url('mssql://username:password@hostspec/database?driver=SQL+Server')
         connection = dialect.create_connect_args(u)
         eq_([['DRIVER={SQL Server};Server=hostspec;Database=database;UI'
             'D=username;PWD=password'], {}], connection)
 
+    def test_pyodbc_host_no_driver(self):
+        dialect = pyodbc.dialect()
+        u = url.make_url('mssql://username:password@hostspec/database')
+
+        def go():
+            return dialect.create_connect_args(u)
+        connection = assert_warnings(
+            go,
+            ["No driver name specified; this is expected by "
+             "PyODBC when using DSN-less connections"])
+
+        eq_([['Server=hostspec;Database=database;UI'
+            'D=username;PWD=password'], {}], connection)
+
     def test_pyodbc_connect_comma_port(self):
         dialect = pyodbc.dialect()
         u = \
             url.make_url('mssql://username:password@hostspec:12345/data'
-                         'base')
+                         'base?driver=SQL Server')
         connection = dialect.create_connect_args(u)
         eq_([['DRIVER={SQL Server};Server=hostspec,12345;Database=datab'
             'ase;UID=username;PWD=password'], {}], connection)
@@ -58,7 +72,7 @@ class ParseConnectTest(fixtures.TestBase):
         dialect = pyodbc.dialect()
         u = \
             url.make_url('mssql://username:password@hostspec/database?p'
-                         'ort=12345')
+                         'ort=12345&driver=SQL+Server')
         connection = dialect.create_connect_args(u)
         eq_([['DRIVER={SQL Server};Server=hostspec;Database=database;UI'
             'D=username;PWD=password;port=12345'], {}], connection)
@@ -67,7 +81,7 @@ class ParseConnectTest(fixtures.TestBase):
         dialect = pyodbc.dialect()
         u = \
             url.make_url('mssql://username:password@hostspec/database?L'
-                         'ANGUAGE=us_english&foo=bar')
+                         'ANGUAGE=us_english&foo=bar&driver=SQL+Server')
         connection = dialect.create_connect_args(u)
         eq_(connection[1], {})
         eq_(connection[0][0]
diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py
index 4977611c5..4f8ced72c 100644
--- a/test/sql/test_compiler.py
+++ b/test/sql/test_compiler.py
@@ -256,7 +256,7 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL):
                             select._offset))
                 return result
 
-            def limit_clause(self, select):
+            def limit_clause(self, select, **kw):
                 return ""
 
         dialect = default.DefaultDialect()