diff options
| -rw-r--r-- | doc/build/changelog/changelog_09.rst | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/compiler.py | 8 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/elements.py | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/naming.py | 38 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/sqltypes.py | 6 | ||||
| -rw-r--r-- | test/sql/test_metadata.py | 86 |
6 files changed, 145 insertions, 25 deletions
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 8be2346f6..24803af35 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -15,6 +15,22 @@ :released: .. change:: + :tags: bug, sql + :tickets: 3067 + :versions: 1.0.0 + + Fix bug in naming convention feature where using a check + constraint convention that includes ``constraint_name`` would + then force all :class:`.Boolean` and :class:`.Enum` types to + require names as well, as these implicitly create a + constraint, even if the ultimate target backend were one that does + not require generation of the constraint such as Postgresql. + The mechanics of naming conventions for these particular + constraints has been reorganized such that the naming + determination is done at DDL compile time, rather than at + constraint/table construction time. + + .. change:: :tags: bug, mssql :tickets: 3025 diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 7a8b07f8f..99b74b3c8 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -2902,7 +2902,13 @@ class IdentifierPreparer(object): def format_savepoint(self, savepoint, name=None): return self.quote(name or savepoint.ident) - def format_constraint(self, constraint): + @util.dependencies("sqlalchemy.sql.naming") + def format_constraint(self, naming, constraint): + if isinstance(constraint.name, elements._defer_name): + name = naming._constraint_name_for_table( + constraint, constraint.table) + if name: + return self.quote(name) return self.quote(constraint.name) def format_table(self, table, use_schema=True, name=None): diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 3882d9e5b..d273d9881 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -3201,6 +3201,22 @@ class _truncated_label(quoted_name): def apply_map(self, map_): return self + +class _defer_name(_truncated_label): + """mark a name as 'deferred' for the purposes of automated name + generation. + + """ + def __new__(cls, value): + if value is None: + return _defer_none_name('_unnamed_') + else: + return super(_defer_name, cls).__new__(cls, value) + + +class _defer_none_name(_defer_name): + """indicate a 'deferred' name that was ultimately the value None.""" + # for backwards compatibility in case # someone is re-implementing the # _truncated_identifier() sequence in a custom diff --git a/lib/sqlalchemy/sql/naming.py b/lib/sqlalchemy/sql/naming.py index d05054bc6..bb838c542 100644 --- a/lib/sqlalchemy/sql/naming.py +++ b/lib/sqlalchemy/sql/naming.py @@ -14,7 +14,7 @@ from .schema import Constraint, ForeignKeyConstraint, PrimaryKeyConstraint, \ UniqueConstraint, CheckConstraint, Index, Table, Column from .. import event, events from .. import exc -from .elements import _truncated_label +from .elements import _truncated_label, _defer_name, _defer_none_name import re class conv(_truncated_label): @@ -77,12 +77,12 @@ class ConventionDict(object): return list(self.const.columns)[idx] def _key_constraint_name(self): - if not self._const_name: + if isinstance(self._const_name, (type(None), _defer_none_name)): raise exc.InvalidRequestError( - "Naming convention including " - "%(constraint_name)s token requires that " - "constraint is explicitly named." - ) + "Naming convention including " + "%(constraint_name)s token requires that " + "constraint is explicitly named." + ) if not isinstance(self._const_name, conv): self.const.name = None return self._const_name @@ -144,6 +144,17 @@ def _get_convention(dict_, key): else: return None +def _constraint_name_for_table(const, table): + metadata = table.metadata + convention = _get_convention(metadata.naming_convention, type(const)) + if convention is not None and ( + const.name is None or not isinstance(const.name, conv) and + "constraint_name" in convention + ): + return conv( + convention % ConventionDict(const, table, + metadata.naming_convention) + ) @event.listens_for(Constraint, "after_parent_attach") @event.listens_for(Index, "after_parent_attach") @@ -156,12 +167,9 @@ def _constraint_name(const, table): lambda col, table: _constraint_name(const, table) ) elif isinstance(table, Table): - metadata = table.metadata - convention = _get_convention(metadata.naming_convention, type(const)) - if convention is not None: - if const.name is None or "constraint_name" in convention: - newname = conv( - convention % ConventionDict(const, table, metadata.naming_convention) - ) - if const.name is None: - const.name = newname + if isinstance(const.name, (conv, _defer_name)): + return + + newname = _constraint_name_for_table(const, table) + if newname is not None: + const.name = newname diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index b4d2d2390..52e5053ef 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -13,7 +13,7 @@ import datetime as dt import codecs from .type_api import TypeEngine, TypeDecorator, to_instance -from .elements import quoted_name, type_coerce +from .elements import quoted_name, type_coerce, _defer_name from .default_comparator import _DefaultColumnComparator from .. import exc, util, processors from .base import _bind_or_error, SchemaEventTarget @@ -1132,7 +1132,7 @@ class Enum(String, SchemaType): e = schema.CheckConstraint( type_coerce(column, self).in_(self.enums), - name=self.name, + name=_defer_name(self.name), _create_rule=util.portable_instancemethod( self._should_create_constraint) ) @@ -1268,7 +1268,7 @@ class Boolean(TypeEngine, SchemaType): e = schema.CheckConstraint( type_coerce(column, self).in_([0, 1]), - name=self.name, + name=_defer_name(self.name), _create_rule=util.portable_instancemethod( self._should_create_constraint) ) diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index 9542711cb..7aa2059dc 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -9,6 +9,7 @@ from sqlalchemy import Integer, String, UniqueConstraint, \ events, Unicode, types as sqltypes, bindparam, \ Table, Column, Boolean, Enum, func, text from sqlalchemy import schema, exc +from sqlalchemy.sql import elements, naming import sqlalchemy as tsa from sqlalchemy.testing import fixtures from sqlalchemy import testing @@ -2811,7 +2812,9 @@ class DialectKWArgTest(fixtures.TestBase): ) -class NamingConventionTest(fixtures.TestBase): +class NamingConventionTest(fixtures.TestBase, AssertsCompiledSQL): + dialect = 'default' + def _fixture(self, naming_convention, table_schema=None): m1 = MetaData(naming_convention=naming_convention) @@ -2831,10 +2834,10 @@ class NamingConventionTest(fixtures.TestBase): uq = UniqueConstraint(u1.c.data) eq_(uq.name, "uq_user_data") - def test_ck_name(self): + def test_ck_name_required(self): u1 = self._fixture(naming_convention={ - "ck": "ck_%(table_name)s_%(constraint_name)s" - }) + "ck": "ck_%(table_name)s_%(constraint_name)s" + }) ck = CheckConstraint(u1.c.data == 'x', name='mycheck') eq_(ck.name, "ck_user_mycheck") @@ -2845,6 +2848,19 @@ class NamingConventionTest(fixtures.TestBase): CheckConstraint, u1.c.data == 'x' ) + def test_ck_name_deferred_required(self): + u1 = self._fixture(naming_convention={ + "ck": "ck_%(table_name)s_%(constraint_name)s" + }) + ck = CheckConstraint(u1.c.data == 'x', name=elements._defer_name(None)) + + assert_raises_message( + exc.InvalidRequestError, + r"Naming convention including %\(constraint_name\)s token " + "requires that constraint is explicitly named.", + schema.AddConstraint(ck).compile + ) + def test_column_attached_ck_name(self): m = MetaData(naming_convention={ "ck": "ck_%(table_name)s_%(constraint_name)s" @@ -2861,6 +2877,17 @@ class NamingConventionTest(fixtures.TestBase): Table('t', m, Column('x', Integer), ck) eq_(ck.name, "ck_t_x1") + def test_uq_name_already_conv(self): + m = MetaData(naming_convention={ + "uq": "uq_%(constraint_name)s_%(column_0_name)s" + }) + + t = Table('mytable', m) + uq = UniqueConstraint(name=naming.conv('my_special_key')) + + t.append_constraint(uq) + eq_(uq.name, "my_special_key") + def test_fk_name_schema(self): u1 = self._fixture(naming_convention={ @@ -2922,9 +2949,18 @@ class NamingConventionTest(fixtures.TestBase): 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, "ck_user_foo" + if isinstance(c, CheckConstraint)][0].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))" + ")" ) def test_schematype_ck_name_enum(self): @@ -2936,7 +2972,45 @@ class NamingConventionTest(fixtures.TestBase): ) eq_( [c for c in u1.constraints - if isinstance(c, CheckConstraint)][0].name, "ck_user_foo" + if isinstance(c, CheckConstraint)][0].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'))" + ")" + ) + + def test_schematype_ck_name_boolean_no_name(self): + m1 = MetaData(naming_convention={ + "ck": "ck_%(table_name)s_%(constraint_name)s" + }) + + u1 = Table( + 'user', m1, + Column('x', Boolean()) + ) + # constraint gets special _defer_none_name + eq_( + [c for c in u1.constraints + if isinstance(c, CheckConstraint)][0].name, "_unnamed_" + ) + # no issue with native boolean + self.assert_compile( + schema.CreateTable(u1), + 'CREATE TABLE "user" (' + "x BOOLEAN" + ")", + dialect='postgresql' + ) + + assert_raises_message( + exc.InvalidRequestError, + "Naming convention including \%\(constraint_name\)s token " + "requires that constraint is explicitly named.", + schema.CreateTable(u1).compile ) def test_ck_constraint_redundant_event(self): |