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_3_24~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=48f7e04d9542e9038d1af0ec3073025f2fba930e;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 (cherry picked from commit 506b88de5e428fd4ad2feff663ba53e2dbb28891) --- 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 e8b5bdb251..a93e3b9153 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -3743,16 +3743,13 @@ class IdentifierPreparer(object): @util.dependencies("sqlalchemy.sql.naming") def format_constraint(self, naming, constraint): - 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 b56bfc04af..c69a9b18c9 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -4498,33 +4498,9 @@ class conv(_truncated_label): __slots__ = () -class _defer_name(_truncated_label): - """Mark a name as 'deferred' for the purposes of automated name - generation. +_NONE_NAME = util.symbol("NONE_NAME") +"""indicate a 'deferred' name that was ultimately the value None.""" - """ - - __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_") # 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 b727b51ce6..e5635b05ba 100644 --- a/lib/sqlalchemy/sql/naming.py +++ b/lib/sqlalchemy/sql/naming.py @@ -12,8 +12,7 @@ import re -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 d4e8507158..3a195e1ec7 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -20,8 +20,8 @@ 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 _literal_as_binds +from .elements import _NONE_NAME from .elements import quoted_name from .elements import Slice from .elements import TypeCoerce as type_coerce # noqa @@ -1598,7 +1598,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}, @@ -1795,7 +1795,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 12723a1af1..43f987e95b 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -38,7 +38,6 @@ from sqlalchemy.schema import AddConstraint from sqlalchemy.schema import CreateIndex from sqlalchemy.schema import DefaultClause from sqlalchemy.schema import DropIndex -from sqlalchemy.sql import elements from sqlalchemy.sql import naming from sqlalchemy.sql.elements import _NONE_NAME from sqlalchemy.sql.elements import literal_column @@ -4774,20 +4773,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)', @@ -4987,7 +4977,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=elements._defer_name(None)) + ck = CheckConstraint(u1.c.data == "x", name=naming._NONE_NAME) assert_raises_message( exc.InvalidRequestError, @@ -5097,15 +5087,21 @@ class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s"} ) - u1 = Table("user", m1, Column("x", Boolean(name="foo"))) - # constraint is not hit - eq_( - [c for c in u1.constraints if isinstance(c, CheckConstraint)][ - 0 - ].name, - "foo", + u1 = Table( + "user", + m1, + Column("x", Boolean(name="foo")), ) - # but is hit at compile time + + self.assert_compile( + schema.CreateTable(u1), + 'CREATE TABLE "user" (' + "x BOOLEAN, " + "CONSTRAINT ck_user_foo CHECK (x IN (0, 1))" + ")", + ) + + # test no side effects from first compile self.assert_compile( schema.CreateTable(u1), 'CREATE TABLE "user" (' @@ -5141,14 +5137,21 @@ 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"))) - eq_( - [c for c in u1.constraints if isinstance(c, CheckConstraint)][ - 0 - ].name, - "foo", + u1 = Table( + "user", + m1, + Column("x", Enum("a", "b", name="foo")), ) - # but is hit at compile time + + self.assert_compile( + schema.CreateTable(u1), + 'CREATE TABLE "user" (' + "x VARCHAR(1), " + "CONSTRAINT ck_user_foo CHECK (x IN ('a', 'b'))" + ")", + ) + + # test no side effects from first compile self.assert_compile( schema.CreateTable(u1), 'CREATE TABLE "user" ('