- SQL Server 2012 now recommends VARCHAR(max), NVARCHAR(max),

VARBINARY(max) for large text/binary types.  The MSSQL dialect will
now respect this based on version detection, as well as the new
``deprecate_large_types`` flag.
fixes #3039

8 files changed, 201 insertions, 24 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 9cc144fc6..6d99095d9 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -23,6 +23,19 @@
     on compatibility concerns, see :doc:`/changelog/migration_10`.
 
     .. change::
+        :tags: feature, mssql
+        :tickets: 3039
+
+        SQL Server 2012 now recommends VARCHAR(max), NVARCHAR(max),
+        VARBINARY(max) for large text/binary types.  The MSSQL dialect will
+        now respect this based on version detection, as well as the new
+        ``deprecate_large_types`` flag.
+
+        .. seealso::
+
+            :ref:`mssql_large_type_deprecation`
+
+    .. change::
         :tags: bug, sqlite
         :tickets: 3257
 
diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst
index 15e066a75..562bb9f1b 100644
--- a/doc/build/changelog/migration_10.rst
+++ b/doc/build/changelog/migration_10.rst
@@ -1619,6 +1619,14 @@ when using ODBC to avoid this issue entirely.
 
 :ticket:`3182`
 
+SQL Server 2012 large text / binary types render as VARCHAR, NVARCHAR, VARBINARY
+--------------------------------------------------------------------------------
+
+The rendering of the :class:`.Text`, :class:`.UnicodeText`, and :class:`.LargeBinary`
+types has been changed for SQL Server 2012 and greater, with options
+to control the behavior completely, based on deprecation guidelines from
+Microsoft.  See :ref:`mssql_large_type_deprecation` for details.
+
 .. _change_3204:
 
 SQLite/Oracle have distinct methods for temporary table/view name reporting
diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index dad02ee0f..5d84975c0 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -226,6 +226,53 @@ The DATE and TIME types are not available for MSSQL 2005 and
 previous - if a server version below 2008 is detected, DDL
 for these types will be issued as DATETIME.
 
+.. _mssql_large_type_deprecation:
+
+Large Text/Binary Type Deprecation
+----------------------------------
+
+Per `SQL Server 2012/2014 Documentation <http://technet.microsoft.com/en-us/library/ms187993.aspx>`_,
+the ``NTEXT``, ``TEXT`` and ``IMAGE`` datatypes are to be removed from SQL Server
+in a future release.   SQLAlchemy normally relates these types to the
+:class:`.UnicodeText`, :class:`.Text` and :class:`.LargeBinary` datatypes.
+
+In order to accommodate this change, a new flag ``deprecate_large_types``
+is added to the dialect, which will be automatically set based on detection
+of the server version in use, if not otherwise set by the user.  The
+behavior of this flag is as follows:
+
+* When this flag is ``True``, the :class:`.UnicodeText`, :class:`.Text` and
+  :class:`.LargeBinary` datatypes, when used to render DDL, will render the
+  types ``NVARCHAR(max)``, ``VARCHAR(max)``, and ``VARBINARY(max)``,
+  respectively.  This is a new behavior as of the addition of this flag.
+
+* When this flag is ``False``, the :class:`.UnicodeText`, :class:`.Text` and
+  :class:`.LargeBinary` datatypes, when used to render DDL, will render the
+  types ``NTEXT``, ``TEXT``, and ``IMAGE``,
+  respectively.  This is the long-standing behavior of these types.
+
+* The flag begins with the value ``None``, before a database connection is
+  established.   If the dialect is used to render DDL without the flag being
+  set, it is interpreted the same as ``False``.
+
+* On first connection, the dialect detects if SQL Server version 2012 or greater
+  is in use; if the flag is still at ``None``, it sets it to ``True`` or
+  ``False`` based on whether 2012 or greater is detected.
+
+* The flag can be set to either ``True`` or ``False`` when the dialect
+  is created, typically via :func:`.create_engine`::
+
+        eng = create_engine("mssql+pymssql://user:pass@host/db",
+                        deprecate_large_types=True)
+
+* Complete control over whether the "old" or "new" types are rendered is
+  available in all SQLAlchemy versions by using the UPPERCASE type objects
+  instead: :class:`.NVARCHAR`, :class:`.VARCHAR`, :class:`.types.VARBINARY`,
+  :class:`.TEXT`, :class:`.mssql.NTEXT`, :class:`.mssql.IMAGE` will always remain
+  fixed and always output exactly that type.
+
+.. versionadded:: 1.0.0
+
 .. _mssql_indexes:
 
 Clustered Index Support
@@ -367,19 +414,20 @@ import operator
 import re
 
 from ... import sql, schema as sa_schema, exc, util
-from ...sql import compiler, expression, \
-    util as sql_util, cast
+from ...sql import compiler, expression, util as sql_util
 from ... import engine
 from ...engine import reflection, default
 from ... import types as sqltypes
 from ...types import INTEGER, BIGINT, SMALLINT, DECIMAL, NUMERIC, \
     FLOAT, TIMESTAMP, DATETIME, DATE, BINARY,\
-    VARBINARY, TEXT, VARCHAR, NVARCHAR, CHAR, NCHAR
+    TEXT, VARCHAR, NVARCHAR, CHAR, NCHAR
 
 
 from ...util import update_wrapper
 from . import information_schema as ischema
 
+# http://sqlserverbuilds.blogspot.com/
+MS_2012_VERSION = (11,)
 MS_2008_VERSION = (10,)
 MS_2005_VERSION = (9,)
 MS_2000_VERSION = (8,)
@@ -545,6 +593,26 @@ class NTEXT(sqltypes.UnicodeText):
     __visit_name__ = 'NTEXT'
 
 
+class VARBINARY(sqltypes.VARBINARY, sqltypes.LargeBinary):
+    """The MSSQL VARBINARY type.
+
+    This type extends both :class:`.types.VARBINARY` and
+    :class:`.types.LargeBinary`.   In "deprecate_large_types" mode,
+    the :class:`.types.LargeBinary` type will produce ``VARBINARY(max)``
+    on SQL Server.
+
+    .. versionadded:: 1.0.0
+
+    .. seealso::
+
+        :ref:`mssql_large_type_deprecation`
+
+
+
+    """
+    __visit_name__ = 'VARBINARY'
+
+
 class IMAGE(sqltypes.LargeBinary):
     __visit_name__ = 'IMAGE'
 
@@ -683,8 +751,17 @@ class MSTypeCompiler(compiler.GenericTypeCompiler):
     def visit_unicode(self, type_):
         return self.visit_NVARCHAR(type_)
 
+    def visit_text(self, type_):
+        if self.dialect.deprecate_large_types:
+            return self.visit_VARCHAR(type_)
+        else:
+            return self.visit_TEXT(type_)
+
     def visit_unicode_text(self, type_):
-        return self.visit_NTEXT(type_)
+        if self.dialect.deprecate_large_types:
+            return self.visit_NVARCHAR(type_)
+        else:
+            return self.visit_NTEXT(type_)
 
     def visit_NTEXT(self, type_):
         return self._extend("NTEXT", type_)
@@ -717,7 +794,10 @@ class MSTypeCompiler(compiler.GenericTypeCompiler):
             return self.visit_TIME(type_)
 
     def visit_large_binary(self, type_):
-        return self.visit_IMAGE(type_)
+        if self.dialect.deprecate_large_types:
+            return self.visit_VARBINARY(type_)
+        else:
+            return self.visit_IMAGE(type_)
 
     def visit_IMAGE(self, type_):
         return "IMAGE"
@@ -1370,13 +1450,15 @@ class MSDialect(default.DefaultDialect):
                  query_timeout=None,
                  use_scope_identity=True,
                  max_identifier_length=None,
-                 schema_name="dbo", **opts):
+                 schema_name="dbo",
+                 deprecate_large_types=None, **opts):
         self.query_timeout = int(query_timeout or 0)
         self.schema_name = schema_name
 
         self.use_scope_identity = use_scope_identity
         self.max_identifier_length = int(max_identifier_length or 0) or \
             self.max_identifier_length
+        self.deprecate_large_types = deprecate_large_types
         super(MSDialect, self).__init__(**opts)
 
     def do_savepoint(self, connection, name):
@@ -1390,6 +1472,9 @@ class MSDialect(default.DefaultDialect):
 
     def initialize(self, connection):
         super(MSDialect, self).initialize(connection)
+        self._setup_version_attributes()
+
+    def _setup_version_attributes(self):
         if self.server_version_info[0] not in list(range(8, 17)):
             # FreeTDS with version 4.2 seems to report here
             # a number like "95.10.255".  Don't know what
@@ -1405,6 +1490,9 @@ class MSDialect(default.DefaultDialect):
             self.implicit_returning = True
         if self.server_version_info >= MS_2008_VERSION:
             self.supports_multivalues_insert = True
+        if self.deprecate_large_types is None:
+            self.deprecate_large_types = \
+                self.server_version_info >= MS_2012_VERSION
 
     def _get_default_schema_name(self, connection):
         if self.server_version_info < MS_2005_VERSION:
@@ -1592,12 +1680,11 @@ class MSDialect(default.DefaultDialect):
             if coltype in (MSString, MSChar, MSNVarchar, MSNChar, MSText,
                            MSNText, MSBinary, MSVarBinary,
                            sqltypes.LargeBinary):
+                if charlen == -1:
+                    charlen = 'max'
                 kwargs['length'] = charlen
                 if collation:
                     kwargs['collation'] = collation
-                if coltype == MSText or \
-                        (coltype in (MSString, MSNVarchar) and charlen == -1):
-                    kwargs.pop('length')
 
             if coltype is None:
                 util.warn(
diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index 94db1d837..9a2de39b4 100644
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -894,7 +894,7 @@ class LargeBinary(_Binary):
 
         :param length: optional, a length for the column for use in
           DDL statements, for those BLOB types that accept a length
-          (i.e. MySQL).  It does *not* produce a small BINARY/VARBINARY
+          (i.e. MySQL).  It does *not* produce a *lengthed* BINARY/VARBINARY
           type - use the BINARY/VARBINARY types specifically for those.
           May be safely omitted if no ``CREATE
           TABLE`` will be issued.  Certain databases may require a
diff --git a/test/dialect/mssql/test_engine.py b/test/dialect/mssql/test_engine.py
index 4b4780d43..a994b1787 100644
--- a/test/dialect/mssql/test_engine.py
+++ b/test/dialect/mssql/test_engine.py
@@ -157,8 +157,7 @@ class ParseConnectTest(fixtures.TestBase):
 
         eq_(dialect.is_disconnect("not an error", None, None), False)
 
-    @testing.only_on(['mssql+pyodbc', 'mssql+pymssql'],
-                            "FreeTDS specific test")
+    @testing.requires.mssql_freetds
     def test_bad_freetds_warning(self):
         engine = engines.testing_engine()
 
diff --git a/test/dialect/mssql/test_reflection.py b/test/dialect/mssql/test_reflection.py
index 0ef69f656..bee441586 100644
--- a/test/dialect/mssql/test_reflection.py
+++ b/test/dialect/mssql/test_reflection.py
@@ -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),
+            Column('test3', types.Text('max')),
             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),
-            Column('test8', types.LargeBinary),
+            Column('test7', types.Text('max')),
+            Column('test8', types.LargeBinary('max')),
             Column('test_passivedefault2', types.Integer,
                    server_default='5'),
             Column('test9', types.BINARY(100)),
@@ -204,6 +204,11 @@ class InfoCoerceUnicodeTest(fixtures.TestBase, AssertsCompiledSQL):
 class ReflectHugeViewTest(fixtures.TestBase):
     __only_on__ = 'mssql'
 
+    # crashes on freetds 0.91, not worth it
+    __skip_if__ = (
+        lambda: testing.requires.mssql_freetds.enabled,
+    )
+
     def setup(self):
         self.col_num = 150
 
diff --git a/test/dialect/mssql/test_types.py b/test/dialect/mssql/test_types.py
index 24f0eb0be..5c9157379 100644
--- a/test/dialect/mssql/test_types.py
+++ b/test/dialect/mssql/test_types.py
@@ -4,7 +4,8 @@ import datetime
 import os
 from sqlalchemy import Table, Column, MetaData, Float, \
     Integer, String, Boolean, TIMESTAMP, Sequence, Numeric, select, \
-    Date, Time, DateTime, DefaultClause, PickleType, text
+    Date, Time, DateTime, DefaultClause, PickleType, text, Text, \
+    UnicodeText, LargeBinary
 from sqlalchemy import types, schema
 from sqlalchemy.databases import mssql
 from sqlalchemy.dialects.mssql.base import TIME
@@ -172,6 +173,44 @@ class TypeDDLTest(fixtures.TestBase):
                 "%s %s" % (col.name, columns[index][3]))
             self.assert_(repr(col))
 
+    def test_large_type_deprecation(self):
+        d1 = mssql.dialect(deprecate_large_types=True)
+        d2 = mssql.dialect(deprecate_large_types=False)
+        d3 = mssql.dialect()
+        d3.server_version_info = (11, 0)
+        d3._setup_version_attributes()
+        d4 = mssql.dialect()
+        d4.server_version_info = (10, 0)
+        d4._setup_version_attributes()
+
+        for dialect in (d1, d3):
+            eq_(
+                str(Text().compile(dialect=dialect)),
+                "VARCHAR(max)"
+            )
+            eq_(
+                str(UnicodeText().compile(dialect=dialect)),
+                "NVARCHAR(max)"
+            )
+            eq_(
+                str(LargeBinary().compile(dialect=dialect)),
+                "VARBINARY(max)"
+            )
+
+        for dialect in (d2, d4):
+            eq_(
+                str(Text().compile(dialect=dialect)),
+                "TEXT"
+            )
+            eq_(
+                str(UnicodeText().compile(dialect=dialect)),
+                "NTEXT"
+            )
+            eq_(
+                str(LargeBinary().compile(dialect=dialect)),
+                "IMAGE"
+            )
+
     def test_timestamp(self):
         """Exercise TIMESTAMP column."""
 
@@ -485,18 +524,18 @@ class TypeRoundTripTest(
 
     @emits_warning_on('mssql+mxodbc', r'.*does not have any indexes.*')
     @testing.provide_metadata
-    def test_binary_reflection(self):
+    def _test_binary_reflection(self, deprecate_large_types):
         "Exercise type specification for binary types."
 
         columns = [
-            # column type, args, kwargs, expected ddl
+            # column type, args, kwargs, expected ddl from reflected
             (mssql.MSBinary, [], {},
-             'BINARY'),
+             'BINARY(1)'),
             (mssql.MSBinary, [10], {},
              'BINARY(10)'),
 
             (types.BINARY, [], {},
-             'BINARY'),
+             'BINARY(1)'),
             (types.BINARY, [10], {},
              'BINARY(10)'),
 
@@ -517,10 +556,12 @@ class TypeRoundTripTest(
              'IMAGE'),
 
             (types.LargeBinary, [], {},
-             'IMAGE'),
+             'IMAGE' if not deprecate_large_types else 'VARBINARY(max)'),
         ]
 
         metadata = self.metadata
+        metadata.bind = engines.testing_engine(
+            options={"deprecate_large_types": deprecate_large_types})
         table_args = ['test_mssql_binary', metadata]
         for index, spec in enumerate(columns):
             type_, args, kw, res = spec
@@ -530,17 +571,29 @@ class TypeRoundTripTest(
         metadata.create_all()
         reflected_binary = Table('test_mssql_binary',
                                  MetaData(testing.db), autoload=True)
-        for col in reflected_binary.c:
+        for col, spec in zip(reflected_binary.c, columns):
+            eq_(
+                str(col.type), spec[3],
+                "column %s %s != %s" % (col.key, str(col.type), spec[3])
+            )
             c1 = testing.db.dialect.type_descriptor(col.type).__class__
             c2 = \
                 testing.db.dialect.type_descriptor(
                     binary_table.c[col.name].type).__class__
-            assert issubclass(c1, c2), '%r is not a subclass of %r' \
-                % (c1, c2)
+            assert issubclass(c1, c2), \
+                'column %s: %r is not a subclass of %r' \
+                % (col.key, c1, c2)
             if binary_table.c[col.name].type.length:
                 testing.eq_(col.type.length,
                             binary_table.c[col.name].type.length)
 
+    def test_binary_reflection_legacy_large_types(self):
+        self._test_binary_reflection(False)
+
+    @testing.only_on('mssql >= 11')
+    def test_binary_reflection_sql2012_large_types(self):
+        self._test_binary_reflection(True)
+
     def test_autoincrement(self):
         Table(
             'ai_1', metadata,
diff --git a/test/requirements.py b/test/requirements.py
index 22ac13fe8..ffbdfba23 100644
--- a/test/requirements.py
+++ b/test/requirements.py
@@ -460,6 +460,7 @@ class DefaultRequirements(SuiteRequirements):
         )
 
 
+
     @property
     def emulated_lastrowid(self):
         """"target dialect retrieves cursor.lastrowid or an equivalent
@@ -777,6 +778,17 @@ class DefaultRequirements(SuiteRequirements):
                 "Not supported on MySQL + Windows"
             )
 
+    @property
+    def mssql_freetds(self):
+        return only_on(
+            LambdaPredicate(
+                lambda config: (
+                    (against(config, 'mssql+pyodbc') and
+                     config.db.dialect.freetds)
+                    or against(config, 'mssql+pymssql')
+                )
+            )
+        )
 
     @property
     def selectone(self):