warn for enum length silently ignored

the "length" parameter is silently ignored when native_enum
is not passed as False.  if native_enum is True, a non-native
VARCHAR can still be generated.   Warn for this silent ignore
right now, consider having "length" used in all cases where
non-native enum is rendered likely in 2.0.

Change-Id: Ibceedd4e3aa3926f3268c0c39d94ab73d17a9bdc

diff --git a/doc/build/changelog/unreleased_14/enum_length_warning.rst b/doc/build/changelog/unreleased_14/enum_length_warning.rst
new file mode 100644 (file)
index 0000000..f1dfab6
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/enum_length_warning.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, sql
+
+    The :class:`_sqltypes.Enum` datatype now emits a warning if the
+    :paramref:`_sqltypes.Enum.length` argument is specified without also
+    specifying :paramref:`_sqltypes.Enum.native_enum` as False, as the
+    parameter is otherwise silently ignored in this case, despite the fact that
+    the :class:`_sqltypes.Enum` datatype will still render VARCHAR DDL on
+    backends that don't have a native ENUM datatype such as SQLite. This
+    behavior may change in a future release so that "length" is honored for all
+    non-native "enum" types regardless of the "native_enum" setting.
+

diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index 6b2d3654b5e237bb857563a485b2e876108c8af2..e103b37098b1631afd1d499f851adf3b9bd466f2 100644 (file)
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -1219,8 +1219,9 @@ class Enum(Emulated, String, TypeEngine[Union[str, enum.Enum]], SchemaType):
 
         :param native_enum: Use the database's native ENUM type when
            available. Defaults to True. When False, uses VARCHAR + check
-           constraint for all backends. The VARCHAR length can be controlled
-           with :paramref:`.Enum.length`
+           constraint for all backends. When False, the VARCHAR length can be
+           controlled with :paramref:`.Enum.length`; currently "length" is
+           ignored if native_enum=True.
 
         :param length: Allows specifying a custom length for the VARCHAR
            when :paramref:`.Enum.native_enum` is False. By default it uses the
@@ -1314,7 +1315,7 @@ class Enum(Emulated, String, TypeEngine[Union[str, enum.Enum]], SchemaType):
         self._sort_key_function = kw.pop("sort_key_function", NO_ARG)
         length_arg = kw.pop("length", NO_ARG)
         self._omit_aliases = kw.pop("omit_aliases", True)
-
+        _disable_warnings = kw.pop("_disable_warnings", False)
         values, objects = self._parse_into_values(enums, kw)
         self._setup_for_values(values, objects, kw)
 
@@ -1324,14 +1325,24 @@ class Enum(Emulated, String, TypeEngine[Union[str, enum.Enum]], SchemaType):
             self._default_length = length = max(len(x) for x in self.enums)
         else:
             self._default_length = length = 0
-        if not self.native_enum and length_arg is not NO_ARG:
-            if length_arg < length:
-                raise ValueError(
-                    "When provided, length must be larger or equal"
-                    " than the length of the longest enum value. %s < %s"
-                    % (length_arg, length)
-                )
-            length = length_arg
+
+        if length_arg is not NO_ARG:
+            if self.native_enum:
+                if not _disable_warnings:
+                    util.warn(
+                        "Enum 'length' argument is currently ignored unless "
+                        "native_enum is specified as False, including for DDL "
+                        "that renders VARCHAR in any case.  This may change "
+                        "in a future release."
+                    )
+            else:
+                if not _disable_warnings and length_arg < length:
+                    raise ValueError(
+                        "When provided, length must be larger or equal"
+                        " than the length of the longest enum value. %s < %s"
+                        % (length_arg, length)
+                    )
+                length = length_arg
 
         self._valid_lookup[None] = self._object_lookup[None] = None
 
@@ -1471,11 +1482,14 @@ class Enum(Emulated, String, TypeEngine[Union[str, enum.Enum]], SchemaType):
                 "an `enums` attribute."
             )
 
-        return util.constructor_copy(self, self._generic_type_affinity, *args)
+        return util.constructor_copy(
+            self, self._generic_type_affinity, *args, _disable_warnings=True
+        )
 
     def adapt_to_emulated(self, impltype, **kw):
         kw.setdefault("validate_strings", self.validate_strings)
         kw.setdefault("name", self.name)
+        kw["_disable_warnings"] = True
         kw.setdefault("schema", self.schema)
         kw.setdefault("inherit_schema", self.inherit_schema)
         kw.setdefault("metadata", self.metadata)
@@ -1490,6 +1504,7 @@ class Enum(Emulated, String, TypeEngine[Union[str, enum.Enum]], SchemaType):
 
     def adapt(self, impltype, **kw):
         kw["_enums"] = self._enums_argument
+        kw["_disable_warnings"] = True
         return super(Enum, self).adapt(impltype, **kw)
 
     def _should_create_constraint(self, compiler, **kw):

diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index d7b9ae6550ee1c340e329973a3bc45f508efac89..a06624b6fe25b1f13e491476f65eacacd4d719de 100644 (file)
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -81,6 +81,7 @@ from sqlalchemy.testing import AssertsExecutionResults
 from sqlalchemy.testing import engines
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import expect_raises
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
 from sqlalchemy.testing import is_not
@@ -2493,13 +2494,30 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest):
             "Enum('x', 'y', native_enum=False, length=255)",
         )
 
+    def test_repr_four(self):
+        with expect_warnings(
+            "Enum 'length' argument is currently ignored unless native_enum"
+        ):
+            e = Enum("x", "y", length=255)
+        # length is currently ignored if native_enum is not False
+        eq_(
+            repr(e),
+            "Enum('x', 'y')",
+        )
+
     def test_length_native(self):
-        e = Enum("x", "y", "long", length=42)
+        with expect_warnings(
+            "Enum 'length' argument is currently ignored unless native_enum"
+        ):
+            e = Enum("x", "y", "long", length=42)
 
         eq_(e.length, len("long"))
 
         # no error is raised
-        e = Enum("x", "y", "long", length=1)
+        with expect_warnings(
+            "Enum 'length' argument is currently ignored unless native_enum"
+        ):
+            e = Enum("x", "y", "long", length=1)
         eq_(e.length, len("long"))
 
     def test_length_raises(self):