From 4955b6f53fa278614d6fd458a899bcb9b75db675 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 4 Mar 2022 18:27:24 -0500 Subject: [PATCH] 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 (cherry picked from commit 18683f474b285b4d7e16c38c0a570276912e1081) --- .../unreleased_14/enum_length_warning.rst | 12 ++++++ lib/sqlalchemy/sql/sqltypes.py | 39 +++++++++++++------ test/sql/test_types.py | 21 +++++++++- 3 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/enum_length_warning.rst 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 index 0000000000..f1dfab64cb --- /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 cc3dbffc5d..0b67e29372 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -1400,8 +1400,9 @@ class Enum(Emulated, String, 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 @@ -1500,7 +1501,7 @@ class Enum(Emulated, String, 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", NO_ARG) - + _disable_warnings = kw.pop("_disable_warnings", False) values, objects = self._parse_into_values(enums, kw) self._setup_for_values(values, objects, kw) @@ -1523,14 +1524,24 @@ class Enum(Emulated, String, 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 @@ -1690,12 +1701,15 @@ class Enum(Emulated, String, 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("_expect_unicode", self._expect_unicode) 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) @@ -1710,6 +1724,7 @@ class Enum(Emulated, String, 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 9f8b8a662c..5384230153 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -2521,13 +2521,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): -- 2.47.2