From: Mike Bayer Date: Mon, 20 Jun 2016 14:08:36 +0000 (-0400) Subject: Disable Enum string validation by default X-Git-Tag: rel_1_1_0b2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=91a1022227499c8efce038c4a0a9bdcdb14a69d0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Disable Enum string validation by default Rolled back the validation rules a bit in :class:`.Enum` to allow unknown string values to pass through, unless the flag ``validate_string=True`` is passed to the Enum; any other kind of object is still of course rejected. While the immediate use is to allow comparisons to enums with LIKE, the fact that this use exists indicates there may be more unknown-string-comparsion use cases than we expected, which hints that perhaps there are some unknown string-INSERT cases too. Change-Id: I7d1d79b374a7d47966d410998f77cd19294ab7b0 Fixes: #3725 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index c2ae4f7cff..abb97229d5 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,19 @@ .. changelog:: :version: 1.1.0b2 + .. change:: + :tags: bug, sql + :tickets: 3725 + + Rolled back the validation rules a bit in :class:`.Enum` to allow + unknown string values to pass through, unless the flag + ``validate_string=True`` is passed to the Enum; any other kind of object is + still of course rejected. While the immediate use + is to allow comparisons to enums with LIKE, the fact that this + use exists indicates there may be more unknown-string-comparsion use + cases than we expected, which hints that perhaps there are some + unknown string-INSERT cases too. + .. changelog:: :version: 1.1.0b1 :released: June 16, 2016 diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index b47614a12c..9772213ef9 100644 --- a/doc/build/changelog/migration_11.rst +++ b/doc/build/changelog/migration_11.rst @@ -1475,13 +1475,14 @@ The ``Enum`` type now does in-Python validation of values To accomodate for Python native enumerated objects, as well as for edge cases such as that of where a non-native ENUM type is used within an ARRAY and a CHECK contraint is infeasible, the :class:`.Enum` datatype now adds -in-Python validation of input values:: +in-Python validation of input values when the :paramref:`.Enum.validate_strings` +flag is used (1.1.0b2):: >>> from sqlalchemy import Table, MetaData, Column, Enum, create_engine >>> t = Table( ... 'data', MetaData(), - ... Column('value', Enum("one", "two", "three")) + ... Column('value', Enum("one", "two", "three", validate_strings=True)) ... ) >>> e = create_engine("sqlite://") >>> t.create(e) @@ -1493,12 +1494,11 @@ in-Python validation of input values:: [SQL: u'INSERT INTO data (value) VALUES (?)'] [parameters: [{'value': 'four'}]] -For simplicity and consistency, this validation is now turned on in all cases, -whether or not the enumerated type uses a database-native form, whether -or not the CHECK constraint is in use, as well as whether or not a -PEP-435 enumerated type or plain list of string values is used. The -check also occurs on the result-handling side as well, when values coming -from the database are returned. +This validation is turned off by default as there are already use cases +identified where users don't want such validation (such as string comparisons). +For non-string types, it necessarily takes place in all cases. The +check also occurs unconditionally on the result-handling side as well, when +values coming from the database are returned. This validation is in addition to the existing behavior of creating a CHECK constraint when a non-native enumerated type is used. The creation of diff --git a/lib/sqlalchemy/dialects/mysql/enumerated.py b/lib/sqlalchemy/dialects/mysql/enumerated.py index 567e95288c..a47d94ca75 100644 --- a/lib/sqlalchemy/dialects/mysql/enumerated.py +++ b/lib/sqlalchemy/dialects/mysql/enumerated.py @@ -114,7 +114,9 @@ class ENUM(sqltypes.Enum, _EnumeratedValues): """ kw.pop('strict', None) - sqltypes.Enum.__init__(self, *enums) + validate_strings = kw.pop("validate_strings", False) + sqltypes.Enum.__init__( + self, validate_strings=validate_strings, *enums) kw.pop('metadata', None) kw.pop('schema', None) kw.pop('name', None) diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 166e618220..9772313365 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -24,6 +24,7 @@ from . import operators from .. import inspection from .. import event from ..util import pickle +from ..util import compat import decimal if util.jython: @@ -1205,6 +1206,11 @@ class Enum(String, SchemaType): ``schema`` attribute. This also takes effect when using the :meth:`.Table.tometadata` operation. + :param validate_strings: when True, invalid string values will + be validated and not be allowed to pass through. + + .. versionadded:: 1.1.0b2 + """ values, objects = self._parse_into_values(enums, kw) @@ -1213,6 +1219,8 @@ class Enum(String, SchemaType): self.native_enum = kw.pop('native_enum', True) convert_unicode = kw.pop('convert_unicode', None) self.create_constraint = kw.pop('create_constraint', True) + self.validate_strings = kw.pop('validate_strings', False) + if convert_unicode is None: for e in self.enums: if isinstance(e, util.text_type): @@ -1262,8 +1270,20 @@ class Enum(String, SchemaType): try: return self._valid_lookup[elem] except KeyError: - raise LookupError( - '"%s" is not among the defined enum values' % elem) + # for unknown string values, we return as is. While we can + # validate these if we wanted, that does not allow for lesser-used + # end-user use cases, such as using a LIKE comparison with an enum, + # or for an application that wishes to apply string tests to an + # ENUM (see [ticket:3725]). While we can decide to differentiate + # here between an INSERT statement and a criteria used in a SELECT, + # for now we're staying conservative w/ behavioral changes (perhaps + # someone has a trigger that handles strings on INSERT) + if not self.validate_strings and \ + isinstance(elem, compat.string_types): + return elem + else: + raise LookupError( + '"%s" is not among the defined enum values' % elem) def _object_value_for_elem(self, elem): try: @@ -1314,6 +1334,7 @@ class Enum(String, SchemaType): convert_unicode=self.convert_unicode, native_enum=self.native_enum, inherit_schema=self.inherit_schema, + validate_strings=self.validate_strings, _create_events=_create_events, *args, **kw) diff --git a/test/dialect/mysql/test_types.py b/test/dialect/mysql/test_types.py index e570e0db1f..7b7cf36679 100644 --- a/test/dialect/mysql/test_types.py +++ b/test/dialect/mysql/test_types.py @@ -666,7 +666,9 @@ class EnumSetTest( 'mysql_enum', self.metadata, Column('e1', e1), Column('e2', e2, nullable=False), - Column('e2generic', Enum("a", "b"), nullable=False), + Column( + 'e2generic', + Enum("a", "b", validate_strings=True), nullable=False), Column('e3', e3), Column('e4', e4, nullable=False), diff --git a/test/sql/test_types.py b/test/sql/test_types.py index 67d20871c2..e540f92464 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -1121,7 +1121,8 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): Column('someenum', Enum('one', 'two', 'three', native_enum=False)), Column('someotherenum', Enum('one', 'two', 'three', - create_constraint=False, native_enum=False)), + create_constraint=False, native_enum=False, + validate_strings=True)), ) Table( @@ -1149,14 +1150,24 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): def test_validators_pep435(self): type_ = Enum(self.SomeEnum) + validate_type = Enum(self.SomeEnum, validate_strings=True) bind_processor = type_.bind_processor(testing.db.dialect) + bind_processor_validates = validate_type.bind_processor( + testing.db.dialect) eq_(bind_processor('one'), "one") eq_(bind_processor(self.one), "one") + eq_(bind_processor("foo"), "foo") + assert_raises_message( + LookupError, + '"5" is not among the defined enum values', + bind_processor, 5 + ) + assert_raises_message( LookupError, '"foo" is not among the defined enum values', - bind_processor, "foo" + bind_processor_validates, "foo" ) result_processor = type_.result_processor(testing.db.dialect, None) @@ -1169,22 +1180,43 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): ) literal_processor = type_.literal_processor(testing.db.dialect) + validate_literal_processor = validate_type.literal_processor( + testing.db.dialect) eq_(literal_processor("one"), "'one'") + + eq_(literal_processor("foo"), "'foo'") + + assert_raises_message( + LookupError, + '"5" is not among the defined enum values', + literal_processor, 5 + ) + assert_raises_message( LookupError, '"foo" is not among the defined enum values', - literal_processor, "foo" + validate_literal_processor, "foo" ) def test_validators_plain(self): type_ = Enum("one", "two") + validate_type = Enum("one", "two", validate_strings=True) bind_processor = type_.bind_processor(testing.db.dialect) + bind_processor_validates = validate_type.bind_processor( + testing.db.dialect) eq_(bind_processor('one'), "one") + eq_(bind_processor('foo'), "foo") + assert_raises_message( + LookupError, + '"5" is not among the defined enum values', + bind_processor, 5 + ) + assert_raises_message( LookupError, '"foo" is not among the defined enum values', - bind_processor, "foo" + bind_processor_validates, "foo" ) result_processor = type_.result_processor(testing.db.dialect, None) @@ -1197,13 +1229,40 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): ) literal_processor = type_.literal_processor(testing.db.dialect) + validate_literal_processor = validate_type.literal_processor( + testing.db.dialect) eq_(literal_processor("one"), "'one'") + eq_(literal_processor("foo"), "'foo'") + assert_raises_message( + LookupError, + '"5" is not among the defined enum values', + literal_processor, 5 + ) + assert_raises_message( LookupError, '"foo" is not among the defined enum values', - literal_processor, "foo" + validate_literal_processor, "foo" ) + def test_validators_not_in_like_roundtrip(self): + enum_table = self.tables['non_native_enum_table'] + + enum_table.insert().execute([ + {'id': 1, 'someenum': 'two'}, + {'id': 2, 'someenum': 'two'}, + {'id': 3, 'someenum': 'one'}, + ]) + + eq_( + enum_table.select(). + where(enum_table.c.someenum.like('%wo%')). + order_by(enum_table.c.id).execute().fetchall(), + [ + (1, 'two', None), + (2, 'two', None), + ] + ) @testing.fails_on( 'postgresql+zxjdbc', @@ -1364,7 +1423,7 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): assert_raises( exc.StatementError, self.tables['non_native_enum_table'].insert().execute, - {'id': 4, 'someenum': 'four'} + {'id': 4, 'someotherenum': 'four'} ) def test_mock_engine_no_prob(self):