From 4399431b53e5d132672431205c654d7d6b32dd77 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 3 Sep 2014 10:30:51 -0400 Subject: - 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. fixes #3182 --- doc/build/changelog/changelog_10.rst | 14 ++++++ doc/build/changelog/migration_10.rst | 18 +++++++ lib/sqlalchemy/connectors/pyodbc.py | 21 ++++++-- lib/sqlalchemy/dialects/mssql/pyodbc.py | 87 +++++++++++++-------------------- test/dialect/mssql/test_engine.py | 26 +++++++--- 5 files changed, 102 insertions(+), 64 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 643035477..715936068 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -21,6 +21,20 @@ series as well. For changes that are specific to 1.0 with an emphasis 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 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/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/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://:@ :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 `_. -* ``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 `_ +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/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] -- cgit v1.2.1 From 2b10aa45a101acfcc6090a3801af998ef8fc1a2d Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 3 Sep 2014 19:42:38 -0400 Subject: - ensure literal_binds works with LIMIT clause, FOR UPDATE --- doc/build/changelog/changelog_10.rst | 6 +++++- lib/sqlalchemy/dialects/mssql/base.py | 2 +- lib/sqlalchemy/dialects/mysql/base.py | 12 ++++++------ lib/sqlalchemy/dialects/oracle/base.py | 6 +++--- lib/sqlalchemy/dialects/postgresql/base.py | 10 +++++----- lib/sqlalchemy/dialects/sqlite/base.py | 10 +++++----- lib/sqlalchemy/dialects/sybase/base.py | 2 +- lib/sqlalchemy/sql/compiler.py | 12 ++++++------ lib/sqlalchemy/testing/suite/test_select.py | 15 +++++++++++++++ test/sql/test_compiler.py | 2 +- 10 files changed, 48 insertions(+), 29 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 715936068..e9b78fe78 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -375,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/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/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 , @@ -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/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() -- cgit v1.2.1