From: Gord Thompson Date: Sat, 6 Mar 2021 19:19:13 +0000 (-0700) Subject: Fix named CHECK constraint name omitted on repeated creates X-Git-Tag: rel_1_4_0~14^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=506b88de5e428fd4ad2feff663ba53e2dbb28891;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fix named CHECK constraint name omitted on repeated creates Fixed issue where the CHECK constraint generated by :class:`_types.Boolean` or :class:`_types.Enum` would fail to render the naming convention correctly after the first compilation, due to an unintended change of state within the name given to the constraint. This issue was first introduced in 0.9 in the fix for issue #3067, and the fix revises the approach taken at that time which appears to have been more involved than what was needed. Co-authored-by: Mike Bayer Fixes: #6007 Change-Id: I7ecff0a9d86191520f6841b3922a5af5a6971fba --- diff --git a/doc/build/changelog/unreleased_13/6007.rst b/doc/build/changelog/unreleased_13/6007.rst new file mode 100644 index 0000000000..6523ba8bd0 --- /dev/null +++ b/doc/build/changelog/unreleased_13/6007.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, sql, sqlite + :tickets: 6007 + + Fixed issue where the CHECK constraint generated by :class:`_types.Boolean` + or :class:`_types.Enum` would fail to render the naming convention + correctly after the first compilation, due to an unintended change of state + within the name given to the constraint. This issue was first introduced in + 0.9 in the fix for issue #3067, and the fix revises the approach taken at + that time which appears to have been more involved than what was needed. \ No newline at end of file diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index c6fa6072ea..d48bca14eb 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -4902,16 +4902,13 @@ class IdentifierPreparer(object): def format_constraint(self, constraint, _alembic_quote=True): naming = util.preloaded.sql_naming - if isinstance(constraint.name, elements._defer_name): + if constraint.name is elements._NONE_NAME: name = naming._constraint_name_for_table( constraint, constraint.table ) if name is None: - if isinstance(constraint.name, elements._defer_none_name): - return None - else: - name = constraint.name + return None else: name = constraint.name diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 85200bf252..b26918f2fe 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -4979,33 +4979,8 @@ class conv(_truncated_label): __slots__ = () -class _defer_name(_truncated_label): - """mark a name as 'deferred' for the purposes of automated name - generation. - - """ - - __slots__ = () - - def __new__(cls, value): - if value is None: - return _NONE_NAME - elif isinstance(value, conv): - return value - else: - return super(_defer_name, cls).__new__(cls, value) - - def __reduce__(self): - return self.__class__, (util.text_type(self),) - - -class _defer_none_name(_defer_name): - """indicate a 'deferred' name that was ultimately the value None.""" - - __slots__ = () - - -_NONE_NAME = _defer_none_name("_unnamed_") +_NONE_NAME = util.symbol("NONE_NAME") +"""indicate a 'deferred' name that was ultimately the value None.""" # for backwards compatibility in case # someone is re-implementing the diff --git a/lib/sqlalchemy/sql/naming.py b/lib/sqlalchemy/sql/naming.py index 005c41683b..b793164352 100644 --- a/lib/sqlalchemy/sql/naming.py +++ b/lib/sqlalchemy/sql/naming.py @@ -13,8 +13,7 @@ import re from . import events # noqa -from .elements import _defer_name -from .elements import _defer_none_name +from .elements import _NONE_NAME from .elements import conv from .schema import CheckConstraint from .schema import Column @@ -57,7 +56,7 @@ class ConventionDict(object): return getattr(col, attrname) def _key_constraint_name(self): - if isinstance(self._const_name, (type(None), _defer_none_name)): + if self._const_name in (None, _NONE_NAME): raise exc.InvalidRequestError( "Naming convention including " "%(constraint_name)s token requires that " @@ -160,14 +159,14 @@ def _constraint_name_for_table(const, table): and ( const.name is None or "constraint_name" in convention - or isinstance(const.name, _defer_name) + or const.name is _NONE_NAME ) ): return conv( convention % ConventionDict(const, table, metadata.naming_convention) ) - elif isinstance(convention, _defer_none_name): + elif convention is _NONE_NAME: return None @@ -203,7 +202,7 @@ def _constraint_name(const, table): ) elif isinstance(table, Table): - if isinstance(const.name, (conv, _defer_name)): + if isinstance(const.name, conv) or const.name is _NONE_NAME: return newname = _constraint_name_for_table(const, table) diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 3484b3ac8b..d075ef77dd 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -22,7 +22,7 @@ from . import type_api from .base import _bind_or_error from .base import NO_ARG from .base import SchemaEventTarget -from .elements import _defer_name +from .elements import _NONE_NAME from .elements import quoted_name from .elements import Slice from .elements import TypeCoerce as type_coerce # noqa @@ -1659,7 +1659,7 @@ class Enum(Emulated, String, SchemaType): e = schema.CheckConstraint( type_coerce(column, self).in_(self.enums), - name=_defer_name(self.name), + name=_NONE_NAME if self.name is None else self.name, _create_rule=util.portable_instancemethod( self._should_create_constraint, {"variant_mapping": variant_mapping}, @@ -1868,7 +1868,7 @@ class Boolean(Emulated, TypeEngine, SchemaType): e = schema.CheckConstraint( type_coerce(column, self).in_([0, 1]), - name=_defer_name(self.name), + name=_NONE_NAME if self.name is None else self.name, _create_rule=util.portable_instancemethod( self._should_create_constraint, {"variant_mapping": variant_mapping}, diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index 2e64e0a906..3e5f7b916b 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -4926,20 +4926,11 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): dialect="default", ) - def test_uq_defer_name_no_convention(self): - u1 = self._fixture(naming_convention={}) - uq = UniqueConstraint(u1.c.data, name=naming._defer_name("myname")) - self.assert_compile( - schema.AddConstraint(uq), - 'ALTER TABLE "user" ADD CONSTRAINT myname UNIQUE (data)', - dialect="default", - ) - def test_uq_defer_name_convention(self): u1 = self._fixture( naming_convention={"uq": "uq_%(table_name)s_%(column_0_name)s"} ) - uq = UniqueConstraint(u1.c.data, name=naming._defer_name("myname")) + uq = UniqueConstraint(u1.c.data, name=naming._NONE_NAME) self.assert_compile( schema.AddConstraint(uq), 'ALTER TABLE "user" ADD CONSTRAINT uq_user_data UNIQUE (data)', @@ -5139,7 +5130,7 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): u1 = self._fixture( naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s"} ) - ck = CheckConstraint(u1.c.data == "x", name=naming._defer_name(None)) + ck = CheckConstraint(u1.c.data == "x", name=naming._NONE_NAME) assert_raises_message( exc.InvalidRequestError, @@ -5254,14 +5245,16 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): m1, Column("x", Boolean(name="foo", create_constraint=True)), ) - # constraint is not hit - eq_( - [c for c in u1.constraints if isinstance(c, CheckConstraint)][ - 0 - ].name, - "foo", + + self.assert_compile( + schema.CreateTable(u1), + 'CREATE TABLE "user" (' + "x BOOLEAN, " + "CONSTRAINT ck_user_foo CHECK (x IN (0, 1))" + ")", ) - # but is hit at compile time + + # test no side effects from first compile self.assert_compile( schema.CreateTable(u1), 'CREATE TABLE "user" (' @@ -5302,13 +5295,16 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): m1, Column("x", Enum("a", "b", name="foo", create_constraint=True)), ) - eq_( - [c for c in u1.constraints if isinstance(c, CheckConstraint)][ - 0 - ].name, - "foo", + + self.assert_compile( + schema.CreateTable(u1), + 'CREATE TABLE "user" (' + "x VARCHAR(1), " + "CONSTRAINT ck_user_foo CHECK (x IN ('a', 'b'))" + ")", ) - # but is hit at compile time + + # test no side effects from first compile self.assert_compile( schema.CreateTable(u1), 'CREATE TABLE "user" ('