]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Fix named CHECK constraint name omitted on repeated creates
authorGord Thompson <gord@gordthompson.com>
Sat, 6 Mar 2021 19:19:13 +0000 (12:19 -0700)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 7 Mar 2021 03:27:59 +0000 (22:27 -0500)
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 <mike_mp@zzzcomputing.com>
Fixes: #6007
Change-Id: I7ecff0a9d86191520f6841b3922a5af5a6971fba

doc/build/changelog/unreleased_13/6007.rst [new file with mode: 0644]
lib/sqlalchemy/sql/compiler.py
lib/sqlalchemy/sql/elements.py
lib/sqlalchemy/sql/naming.py
lib/sqlalchemy/sql/sqltypes.py
test/sql/test_metadata.py

diff --git a/doc/build/changelog/unreleased_13/6007.rst b/doc/build/changelog/unreleased_13/6007.rst
new file mode 100644 (file)
index 0000000..6523ba8
--- /dev/null
@@ -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
index c6fa6072ea32b664e9029a2b09136c38226aef3f..d48bca14eb77e8fe1f688aacdeb05a280459a73d 100644 (file)
@@ -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
 
index 85200bf25210dbb4cdeed2fb002968e562fa01e6..b26918f2fe684c76fd31991982ce9f3046dc441b 100644 (file)
@@ -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
index 005c41683b59cd0e2c2fdd6e060f4bc5cd414e6c..b7931643522d55d622f155900126777fec1d685f 100644 (file)
@@ -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)
index 3484b3ac8b8421ef12d5f4eef23752acc92de7fb..d075ef77dd01ff6178b726e2aebe030da4449064 100644 (file)
@@ -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},
index 2e64e0a9064fe2494fe81c4ef5f43e815141d517..3e5f7b916bc773ffcf3a2841d2dc8d76f6d12de7 100644 (file)
@@ -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" ('