- 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

4 files changed, 83 insertions, 5 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 1f18d6d16..bb395a826 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 9c046ab9e..f5602a8ad 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
+    ...
+    <class 'sqlalchemy.sql.sqltypes.VARCHAR'> max
+    <class 'sqlalchemy.dialects.mssql.base.VARBINARY'> 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
+    ...
+    <class 'sqlalchemy.sql.sqltypes.VARCHAR'> None
+    <class 'sqlalchemy.dialects.mssql.base.VARBINARY'> 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 bd41c19bf..6670a28bd 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 bee441586..daf8af724 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