From: Mike Bayer Date: Fri, 4 Mar 2022 23:27:24 +0000 (-0500) Subject: warn for enum length silently ignored X-Git-Tag: rel_2_0_0b1~445^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=18683f474b285b4d7e16c38c0a570276912e1081;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 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 6b2d3654b5..e103b37098 100644 --- 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 d7b9ae6550..a06624b6fe 100644 --- 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):