From dd6110eed335154e0ae14b2dba13e44af76c4f2b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 30 Jul 2015 10:34:36 -0400 Subject: [PATCH] - Fixed issue where the SQL Server dialect would reflect a string- or other variable-length column type with unbounded length by assigning the token ``"max"`` to the length attribute of the string. While using the ``"max"`` token explicitly is supported by the SQL Server dialect, it isn't part of the normal contract of the base string types, and instead the length should just be left as None. The dialect now assigns the length to None on reflection of the type so that the type behaves normally in other contexts. fixes #3504 --- doc/build/changelog/changelog_11.rst | 17 +++++++++++++ doc/build/changelog/migration_11.rst | 35 +++++++++++++++++++++++++++ lib/sqlalchemy/dialects/mssql/base.py | 2 +- test/dialect/mssql/test_reflection.py | 34 +++++++++++++++++++++++--- 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 1f18d6d16d..bb395a826c 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,3 +21,20 @@ .. changelog:: :version: 1.1.0b1 + .. change:: + :tags: bug, mssql + :tickets: 3504 + + Fixed issue where the SQL Server dialect would reflect a string- + or other variable-length column type with unbounded length + by assigning the token ``"max"`` to the + length attribute of the string. While using the ``"max"`` token + explicitly is supported by the SQL Server dialect, it isn't part + of the normal contract of the base string types, and instead the + length should just be left as None. The dialect now assigns the + length to None on reflection of the type so that the type behaves + normally in other contexts. + + .. seealso:: + + :ref:`change_3504` \ No newline at end of file diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index 9c046ab9e2..f5602a8ad9 100644 --- a/doc/build/changelog/migration_11.rst +++ b/doc/build/changelog/migration_11.rst @@ -94,6 +94,41 @@ Dialect Improvements and Changes - SQLite Dialect Improvements and Changes - SQL Server ============================================= +.. _change_3504: + +String / varlength types no longer represent "max" explicitly on reflection +--------------------------------------------------------------------------- + +When reflecting a type such as :class:`.String`, :class:`.Text`, etc. +which includes a length, an "un-lengthed" type under SQL Server would +copy the "length" parameter as the value ``"max"``:: + + >>> from sqlalchemy import create_engine, inspect + >>> engine = create_engine('mssql+pyodbc://scott:tiger@ms_2008', echo=True) + >>> engine.execute("create table s (x varchar(max), y varbinary(max))") + >>> insp = inspect(engine) + >>> for col in insp.get_columns("s"): + ... print col['type'].__class__, col['type'].length + ... + max + max + +The "length" parameter in the base types is expected to be an integer value +or None only; None indicates unbounded length which the SQL Server dialect +interprets as "max". The fix then is so that these lengths come +out as None, so that the type objects work in non-SQL Server contexts:: + + >>> for col in insp.get_columns("s"): + ... print col['type'].__class__, col['type'].length + ... + None + None + +Applications which may have been relying on a direct comparison of the "length" +value to the string "max" should consider the value of ``None`` to mean +the same thing. + +:ticket:`3504` Dialect Improvements and Changes - Oracle ============================================= diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index bd41c19bfd..6670a28bdc 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -1772,7 +1772,7 @@ class MSDialect(default.DefaultDialect): MSNText, MSBinary, MSVarBinary, sqltypes.LargeBinary): if charlen == -1: - charlen = 'max' + charlen = None kwargs['length'] = charlen if collation: kwargs['collation'] = collation diff --git a/test/dialect/mssql/test_reflection.py b/test/dialect/mssql/test_reflection.py index bee4415865..daf8af724e 100644 --- a/test/dialect/mssql/test_reflection.py +++ b/test/dialect/mssql/test_reflection.py @@ -1,5 +1,5 @@ # -*- encoding: utf-8 -from sqlalchemy.testing import eq_ +from sqlalchemy.testing import eq_, is_, in_ from sqlalchemy import * from sqlalchemy import types, schema, event from sqlalchemy.databases import mssql @@ -24,14 +24,14 @@ class ReflectionTest(fixtures.TestBase, ComparesTables): Column('user_name', types.VARCHAR(20), nullable=False), Column('test1', types.CHAR(5), nullable=False), Column('test2', types.Float(5), nullable=False), - Column('test3', types.Text('max')), + Column('test3', types.Text()), Column('test4', types.Numeric, nullable=False), Column('test5', types.DateTime), Column('parent_user_id', types.Integer, ForeignKey('engine_users.user_id')), Column('test6', types.DateTime, nullable=False), - Column('test7', types.Text('max')), - Column('test8', types.LargeBinary('max')), + Column('test7', types.Text()), + Column('test8', types.LargeBinary()), Column('test_passivedefault2', types.Integer, server_default='5'), Column('test9', types.BINARY(100)), @@ -171,6 +171,32 @@ class ReflectionTest(fixtures.TestBase, ComparesTables): set([t2.c['x col'], t2.c.y]) ) + @testing.provide_metadata + def test_max_ident_in_varchar_not_present(self): + """test [ticket:3504]. + + Here we are testing not just that the "max" token comes back + as None, but also that these types accept "max" as the value + of "length" on construction, which isn't a directly documented + pattern however is likely in common use. + + """ + metadata = self.metadata + + Table( + 't', metadata, + Column('t1', types.String), + Column('t2', types.Text('max')), + Column('t3', types.Text('max')), + Column('t4', types.LargeBinary('max')), + Column('t5', types.VARBINARY('max')), + ) + metadata.create_all() + for col in inspect(testing.db).get_columns('t'): + is_(col['type'].length, None) + in_('max', str(col['type'].compile(dialect=testing.db.dialect))) + + from sqlalchemy.dialects.mssql.information_schema import CoerceUnicode, tables from sqlalchemy.dialects.mssql import base -- 2.47.3