From: Mike Bayer Date: Tue, 2 Jun 2020 18:21:03 +0000 (-0400) Subject: Default create_constraint to False X-Git-Tag: rel_1_4_0b1~281^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=97cd0a5db8bb2e47f38899592740d1bc75ec0412;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Default create_constraint to False The :paramref:`.Enum.create_constraint` and :paramref:`.Boolean.create_constraint` parameters now default to False, indicating when a so-called "non-native" version of these two datatypes is created, a CHECK constraint will not be generated by default. These CHECK constraints present schema-management maintenance complexities that should be opted in to, rather than being turned on by default. Fixes: #5367 Change-Id: I0a3fb608ce32143fa757546cc17ba2013e93272a --- diff --git a/doc/build/changelog/unreleased_14/5367.rst b/doc/build/changelog/unreleased_14/5367.rst new file mode 100644 index 0000000000..c2d6f0cbff --- /dev/null +++ b/doc/build/changelog/unreleased_14/5367.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, schema + :tickets: 5367 + + The :paramref:`.Enum.create_constraint` and + :paramref:`.Boolean.create_constraint` parameters now default to False, + indicating when a so-called "non-native" version of these two datatypes is + created, a CHECK constraint will not be generated by default. These CHECK + constraints present schema-management maintenance complexities that should + be opted in to, rather than being turned on by default. + diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 8684a79226..732b775f62 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -1231,11 +1231,10 @@ class Enum(Emulated, String, SchemaType): which the column is constrained towards. The :class:`.Enum` type will make use of the backend's native "ENUM" - type if one is available; otherwise, it uses a VARCHAR datatype and - produces a CHECK constraint. Use of the backend-native enum type - can be disabled using the :paramref:`.Enum.native_enum` flag, and - the production of the CHECK constraint is configurable using the - :paramref:`.Enum.create_constraint` flag. + type if one is available; otherwise, it uses a VARCHAR datatype. + An option also exists to automatically produce a CHECK constraint + when the VARCHAR (so called "non-native") variant is produced; + see the :paramref:`.Enum.create_constraint` flag. The :class:`.Enum` type also provides in-Python validation of string values during both read and write operations. When reading a value @@ -1329,13 +1328,20 @@ class Enum(Emulated, String, SchemaType): result-set processing for this Enum's data. This is set automatically based on the presence of unicode label strings. - :param create_constraint: defaults to True. When creating a non-native - enumerated type, also build a CHECK constraint on the database - against the valid values. + :param create_constraint: defaults to False. When creating a + non-native enumerated type, also build a CHECK constraint on the + database against the valid values. - .. versionadded:: 1.1 - added :paramref:`.Enum.create_constraint` - which provides the option to disable the production of the - CHECK constraint for a non-native enumerated type. + .. note:: it is strongly recommended that the CHECK constraint + have an explicit name in order to support schema-management + concerns. This can be established either by setting the + :paramref:`.Enum.name` parameter or by setting up an + appropriate naming convention; see + :ref:`constraint_naming_conventions` for background. + + .. versionchanged:: 1.4 - this flag now defaults to False, meaning + no CHECK constraint is generated for a non-native enumerated + type. :param metadata: Associate this type directly with a ``MetaData`` object. For types that exist on the target database as an @@ -1434,7 +1440,7 @@ class Enum(Emulated, String, SchemaType): """ self.native_enum = kw.pop("native_enum", True) - self.create_constraint = kw.pop("create_constraint", True) + self.create_constraint = kw.pop("create_constraint", False) self.values_callable = kw.pop("values_callable", None) self._sort_key_function = kw.pop("sort_key_function", NO_ARG) length_arg = kw.pop("length", NO_ARG) @@ -1774,10 +1780,8 @@ class Boolean(Emulated, TypeEngine, SchemaType): that the values persisted are simple true/false values. For all backends, only the Python values ``None``, ``True``, ``False``, ``1`` or ``0`` are accepted as parameter values. For those backends that - don't support a "native boolean" datatype, a CHECK constraint is also - created on the target column. Production of the CHECK constraint - can be disabled by passing the :paramref:`.Boolean.create_constraint` - flag set to ``False``. + don't support a "native boolean" datatype, an option exists to + also create a CHECK constraint on the target column .. versionchanged:: 1.2 the :class:`.Boolean` datatype now asserts that incoming Python values are already in pure boolean form. @@ -1788,13 +1792,28 @@ class Boolean(Emulated, TypeEngine, SchemaType): __visit_name__ = "boolean" native = True - def __init__(self, create_constraint=True, name=None, _create_events=True): + def __init__( + self, create_constraint=False, name=None, _create_events=True + ): """Construct a Boolean. - :param create_constraint: defaults to True. If the boolean + :param create_constraint: defaults to False. If the boolean is generated as an int/smallint, also create a CHECK constraint on the table that ensures 1 or 0 as a value. + .. note:: it is strongly recommended that the CHECK constraint + have an explicit name in order to support schema-management + concerns. This can be established either by setting the + :paramref:`.Boolean.name` parameter or by setting up an + appropriate naming convention; see + :ref:`constraint_naming_conventions` for background. + + .. versionchanged:: 1.4 - this flag now defaults to False, meaning + no CHECK constraint is generated for a non-native enumerated + type. + + + :param name: if a CHECK constraint is generated, specify the name of the constraint. diff --git a/test/dialect/mysql/test_types.py b/test/dialect/mysql/test_types.py index 009d1b26e8..de18c8e9d1 100644 --- a/test/dialect/mysql/test_types.py +++ b/test/dialect/mysql/test_types.py @@ -1233,7 +1233,10 @@ class EnumSetTest( t1 = Table( "sometable", MetaData(), - Column("somecolumn", Enum("x", "y", "z", native_enum=False)), + Column( + "somecolumn", + Enum("x", "y", "z", native_enum=False, create_constraint=True), + ), ) self.assert_compile( schema.CreateTable(t1), diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py index c707137a81..2223b0a76f 100644 --- a/test/dialect/postgresql/test_compiler.py +++ b/test/dialect/postgresql/test_compiler.py @@ -212,7 +212,10 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): t1 = Table( "sometable", MetaData(), - Column("somecolumn", Enum("x", "y", "z", native_enum=False)), + Column( + "somecolumn", + Enum("x", "y", "z", native_enum=False, create_constraint=True), + ), ) self.assert_compile( schema.CreateTable(t1), diff --git a/test/dialect/postgresql/test_types.py b/test/dialect/postgresql/test_types.py index 2adde8edc3..d7f6faf925 100644 --- a/test/dialect/postgresql/test_types.py +++ b/test/dialect/postgresql/test_types.py @@ -288,7 +288,14 @@ class EnumTest(fixtures.TestBase, AssertsExecutionResults): metadata, Column( "bar", - Enum("one", "two", "three", name="myenum", native_enum=False), + Enum( + "one", + "two", + "three", + name="myenum", + create_constraint=True, + native_enum=False, + ), ), ) @@ -317,7 +324,14 @@ class EnumTest(fixtures.TestBase, AssertsExecutionResults): "foo", metadata, Column( - "bar", Enum("B", util.u("Ü"), name="myenum", native_enum=False) + "bar", + Enum( + "B", + util.u("Ü"), + name="myenum", + create_constraint=True, + native_enum=False, + ), ), ) @@ -500,7 +514,16 @@ class EnumTest(fixtures.TestBase, AssertsExecutionResults): t1 = Table( "foo", metadata, - Column("bar", Enum("one", "two", "three", name="myenum")), + Column( + "bar", + Enum( + "one", + "two", + "three", + name="myenum", + create_constraint=True, + ), + ), ) def go(): @@ -1762,7 +1785,12 @@ class ArrayEnum(fixtures.TestBase): "my_col", array_cls( enum_cls( - "foo", "bar", "baz", name="my_enum", native_enum=False + "foo", + "bar", + "baz", + name="my_enum", + create_constraint=True, + native_enum=False, ) ), ), diff --git a/test/dialect/test_sqlite.py b/test/dialect/test_sqlite.py index 04a15d5878..fabbfa4a41 100644 --- a/test/dialect/test_sqlite.py +++ b/test/dialect/test_sqlite.py @@ -1065,7 +1065,13 @@ class SQLTest(fixtures.TestBase, AssertsCompiledSQL): def test_column_defaults_ddl(self): t = Table( - "t", MetaData(), Column("x", Boolean, server_default=sql.false()) + "t", + MetaData(), + Column( + "x", + Boolean(create_constraint=True), + server_default=sql.false(), + ), ) self.assert_compile( diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index a75aee6e99..2145e72d09 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -2258,7 +2258,7 @@ class SchemaTypeTest(fixtures.TestBase): def test_boolean_constraint_type_doesnt_double(self): m1 = MetaData() - t1 = Table("x", m1, Column("flag", Boolean())) + t1 = Table("x", m1, Column("flag", Boolean(create_constraint=True))) eq_( len([c for c in t1.constraints if isinstance(c, CheckConstraint)]), 1, @@ -2274,7 +2274,11 @@ class SchemaTypeTest(fixtures.TestBase): def test_enum_constraint_type_doesnt_double(self): m1 = MetaData() - t1 = Table("x", m1, Column("flag", Enum("a", "b", "c"))) + t1 = Table( + "x", + m1, + Column("flag", Enum("a", "b", "c", create_constraint=True)), + ) eq_( len([c for c in t1.constraints if isinstance(c, CheckConstraint)]), 1, @@ -5031,7 +5035,11 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s"} ) - u1 = Table("user", m1, Column("x", Boolean(name="foo"))) + u1 = Table( + "user", + m1, + Column("x", Boolean(name="foo", create_constraint=True)), + ) # constraint is not hit eq_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ @@ -5053,7 +5061,7 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): naming_convention={"ck": "ck_%(table_name)s_%(column_0_name)s"} ) - u1 = Table("user", m1, Column("x", Boolean())) + u1 = Table("user", m1, Column("x", Boolean(create_constraint=True))) # constraint is not hit is_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ @@ -5075,7 +5083,11 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s"} ) - u1 = Table("user", m1, Column("x", Enum("a", "b", name="foo"))) + u1 = Table( + "user", + m1, + Column("x", Enum("a", "b", name="foo", create_constraint=True)), + ) eq_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ 0 @@ -5097,7 +5109,14 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): ) u1 = Table( - "user", m1, Column("x", Enum("a", "b", name=naming.conv("foo"))) + "user", + m1, + Column( + "x", + Enum( + "a", "b", name=naming.conv("foo"), create_constraint=True + ), + ), ) eq_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ @@ -5119,7 +5138,7 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s"} ) - u1 = Table("user", m1, Column("x", Boolean())) + u1 = Table("user", m1, Column("x", Boolean(create_constraint=True))) # constraint gets special _defer_none_name is_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ @@ -5145,7 +5164,7 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): def test_schematype_no_ck_name_boolean_no_name(self): m1 = MetaData() # no naming convention - u1 = Table("user", m1, Column("x", Boolean())) + u1 = Table("user", m1, Column("x", Boolean(create_constraint=True))) # constraint gets special _defer_none_name is_( [c for c in u1.constraints if isinstance(c, CheckConstraint)][ diff --git a/test/sql/test_types.py b/test/sql/test_types.py index a0c4ee0220..30e5d1fca1 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -1331,25 +1331,43 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): @classmethod def define_tables(cls, metadata): + # note create_constraint has changed in 1.4 as of #5367 Table( "enum_table", metadata, Column("id", Integer, primary_key=True), - Column("someenum", Enum("one", "two", "three", name="myenum")), + Column( + "someenum", + Enum( + "one", + "two", + "three", + name="myenum", + create_constraint=True, + ), + ), ) Table( "non_native_enum_table", metadata, Column("id", Integer, primary_key=True, autoincrement=False), - Column("someenum", Enum("one", "two", "three", native_enum=False)), + Column( + "someenum", + Enum( + "one", + "two", + "three", + native_enum=False, + create_constraint=True, + ), + ), Column( "someotherenum", Enum( "one", "two", "three", - create_constraint=False, native_enum=False, validate_strings=True, ), @@ -1360,7 +1378,7 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): "stdlib_enum_table", metadata, Column("id", Integer, primary_key=True), - Column("someenum", Enum(cls.SomeEnum)), + Column("someenum", Enum(cls.SomeEnum, create_constraint=True)), ) Table( @@ -1372,6 +1390,7 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): Enum( cls.SomeOtherEnum, values_callable=EnumTest.get_enum_string_values, + create_constraint=True, ), ), ) @@ -1605,9 +1624,21 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): Column( "data", Enum( - "one", "two", "three", native_enum=False, name="e1" + "one", + "two", + "three", + native_enum=False, + name="e1", + create_constraint=True, ).with_variant( - Enum("four", "five", "six", native_enum=False, name="e2"), + Enum( + "four", + "five", + "six", + native_enum=False, + name="e2", + create_constraint=True, + ), "some_other_db", ), ), @@ -1638,9 +1669,21 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): Column( "data", Enum( - "one", "two", "three", native_enum=False, name="e1" + "one", + "two", + "three", + native_enum=False, + name="e1", + create_constraint=True, ).with_variant( - Enum("four", "five", "six", native_enum=False, name="e2"), + Enum( + "four", + "five", + "six", + native_enum=False, + name="e2", + create_constraint=True, + ), testing.db.dialect.name, ), ), @@ -1896,7 +1939,10 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): class MyEnum(TypeDecorator): def __init__(self, values): self.impl = Enum( - *[v.name for v in values], name="myenum", native_enum=False + *[v.name for v in values], + name="myenum", + native_enum=False, + create_constraint=True ) # future method @@ -1931,7 +1977,11 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest): are created with checkfirst=False""" e = engines.mock_engine() - t = Table("t1", MetaData(), Column("x", Enum("x", "y", name="pge"))) + t = Table( + "t1", + MetaData(), + Column("x", Enum("x", "y", name="pge", create_constraint=True)), + ) t.create(e, checkfirst=False) # basically looking for the start of # the constraint, or the ENUM def itself, @@ -3038,6 +3088,9 @@ class BooleanTest( """test edge cases for booleans. Note that the main boolean test suite is now in testing/suite/test_types.py + the default value of create_constraint was changed to False in + version 1.4 with #5367. + """ __backend__ = True @@ -3048,8 +3101,8 @@ class BooleanTest( "boolean_table", metadata, Column("id", Integer, primary_key=True, autoincrement=False), - Column("value", Boolean), - Column("unconstrained_value", Boolean(create_constraint=False)), + Column("value", Boolean(create_constraint=True)), + Column("unconstrained_value", Boolean()), ) @testing.fails_on( @@ -3078,7 +3131,7 @@ class BooleanTest( self.value = value class MyBool(TypeDecorator): - impl = Boolean() + impl = Boolean(create_constraint=True) # future method def process_literal_param(self, value, dialect):