Skip to content

Commit

Permalink
Fixes sqlalchemy#4784 regarding constraints:
Browse files Browse the repository at this point in the history
* introduced new type_ck prefix for type-bound constraints; code and docs
* misc unit tests to cover the type-bound items
* added unit tests to ensure pep-compliant enum objects are used to generate a name as the docs instruct

Fixes sqlalchemy#4790 regarding exception text:
* added some debug info to constraint naming exceptions

Change-Id: I48dd77301c10d6462847e2d00ac1412be20a74b2
  • Loading branch information
jvanasco committed Sep 11, 2020
1 parent e78c8f8 commit b7047f9
Show file tree
Hide file tree
Showing 6 changed files with 518 additions and 44 deletions.
13 changes: 13 additions & 0 deletions doc/build/changelog/unreleased_14/4784.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.. change::
:tags: feature, orm
:tickets: 4784, 4790

Added support for a new :paramref:`_schema.MetaData.naming_convention`,
`type_ck`.

The `CheckConstraint` created by `Boolean` and `Enum` columns is a so-called
"type-bound" constraint, and often needs a different naming convention than
other constraints.

Exception messages generated by `create_table` when there related errors are
also improved.
46 changes: 36 additions & 10 deletions doc/build/core/constraints.rst
Original file line number Diff line number Diff line change
Expand Up @@ -454,18 +454,19 @@ event-based approach is included, and can be configured using the argument
the :class:`.Index` class or individual :class:`.Constraint` classes as keys,
and Python string templates as values. It also accepts a series of
string-codes as alternative keys, ``"fk"``, ``"pk"``,
``"ix"``, ``"ck"``, ``"uq"`` for foreign key, primary key, index,
check, and unique constraint, respectively. The string templates in this
dictionary are used whenever a constraint or index is associated with this
:class:`_schema.MetaData` object that does not have an existing name given (including
one exception case where an existing name can be further embellished).
``"ix"``, ``"ck"``, ``"type_ck"``, ``"uq"`` for foreign key, primary key, index,
check, "type bound" check, and unique constraint, respectively. The string templates
in this dictionary are used whenever a constraint or index is associated with this
:class:`._schema.MetaData` object that does not have an existing name given
(including one exception case where an existing name can be further embellished).

An example naming convention that suits basic cases is as follows::

convention = {
"ix": 'ix_%(column_0_label)s',
"uq": "uq_%(table_name)s_%(column_0_name)s",
"ck": "ck_%(table_name)s_%(constraint_name)s",
"type_ck": "ck_%(table_name)s_%(column_0_name)s",
"fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
"pk": "pk_%(table_name)s"
}
Expand Down Expand Up @@ -683,9 +684,21 @@ one column present, the scan does use a deterministic search, however the
structure of the expression will determine which column is noted as
"column zero".

Some backends do not natively support core SqlAlchemy datatypes like
:class:`.Boolean` and :class:`.Enum`, so SqlAlchemy enforces standard
behaviors by automatically generating a CHECK constraint. These "Type Bound"
column types will prefer the specialized ``type_ck`` naming convention when
available and needed. The ``type_ck`` prefix is only used for auto-generation;
if a constraint name is explicitly provided, the naming convention with a ``ck``
prefix will be used. This topic is covered in the next section.

.. versionadded:: 1.0.0 The :class:`.CheckConstraint` object now supports
the ``column_0_name`` naming convention token.

.. seealso::

:ref:`_naming_schematypes`

.. _naming_schematypes:

Configuring Naming for Boolean, Enum, and other schema types
Expand Down Expand Up @@ -730,23 +743,36 @@ MySQL.

The CHECK constraint may also make use of the ``column_0_name`` token,
which works nicely with :class:`.SchemaType` since these constraints have
only one column::
only one column. The ``column_0_name`` token is the recommended token when
dealing with auto-generated "type bound" constraints if you do not want to
explicitly name them::

metadata = MetaData(
naming_convention={"ck": "ck_%(table_name)s_%(column_0_name)s"}
naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s",
"type_ck": "ck_%(table_name)s_%(column_0_name)s",
}
)

Table('foo', metadata,
Column('flag', Boolean())
Column('flag_explicit', Boolean(name='explicit_flag_constraint')),
Column('flag_auto', Boolean()),
)

The above schema will produce::

CREATE TABLE foo (
flag BOOL,
CONSTRAINT ck_foo_flag CHECK (flag IN (0, 1))
flag_explicit BOOL,
flag_auto BOOL,
CONSTRAINT ck_foo_explicit_flag_constraint CHECK (flag_explicit IN (0, 1)),
CONSTRAINT ck_foo_flag_auto CHECK (flag_auto IN (0, 1)),
)

Note how the type-bound :class:`.Boolean` created a contraint name using the
"type_ck" naming convention template when no constraint name was provided, and
created a constraint with the "ck" naming convention when a name was provided.

.. versionchanged:: 1.4 Introduced the special ``type_ck`` prefix.

.. versionchanged:: 1.0 Constraint naming conventions that don't include
``%(constraint_name)s`` again work with :class:`.SchemaType` constraints.

Expand Down
52 changes: 52 additions & 0 deletions doc/build/errors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,58 @@ Since "b" is required, pass it as ``None`` so that the INSERT may proceed::

:ref:`execute_multiple`


.. _error_f0f1:

Naming convention "ck_%(table_name)s_%(constraint_name)s" failed to apply to CHECK constraint...
------------------------------------------------------------------------------------------------

This error typically occurs when SqlAlchemy must automatically generate a check
constraint for a "type bound" column such as :class:`.Boolean` or :class:`.Enum`,
and the naming convention references a `constraint_name`, but a constraint
name was not explicitly provided. SqlAlchemy only generates type-bound constraints
on certain database backends which do not have native enforcement of data types.

The simplest way to address this issue is to update your MetaData's naming convention
to include a ``type_ck`` prefix and corresponding template which uses the
``column_0_name`` token. This naming convention will only be used when automatically
generating contraints for "type bound" columns. The "type_ck"``" prefix was added
in SqlAlchemy 1.4 .
This following example adds a "type_ck" prefix and template which use the
``column_0_name`` token instead of ``constraint_name`` ::

metadata = MetaData(
naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s",
"type_ck": "ck_%(table_name)s_%(column_0_name)s",
}
)
Table('foo', metadata,
Column('flag', Boolean())
)

If you prefer more control over your constraint naming, SqlAlchemy allows you
to supply the contraint name in the column definition itself, using the `name`
argument ::

metadata = MetaData(
naming_convention={"ck": "ck_%(table_name)s_%(constraint_name)s",
}
)
Table('foo', metadata,
Column('flag', Boolean(name='flag_bool'))
)

In the example above, because there is a contraint name explicitly supplied to the
:class:`.Boolean` column, the "ck_" prefix can utilize the "%(constraint_name)s"
token and a "type_ck" template is not needed.


.. seealso::

:ref:`_naming_schematypes`


.. _error_89ve:

Expected FROM clause, got Select. To create a FROM clause, use the .subquery() method
Expand Down
110 changes: 92 additions & 18 deletions lib/sqlalchemy/sql/naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,31 @@


class ConventionDict(object):
def __init__(self, const, table, convention):
def __init__(self, const, table, naming_convention, active_convention):
"""
`naming_convention`:
a dict of naming conventions, used as an arg to Metadata
`active_convention`:
the active convention/key being populated by this instance.
this is only needed for detailed debug information in exceptions
"""
self.const = const
self._is_fk = isinstance(const, ForeignKeyConstraint)
self.table = table
self.convention = convention
self.naming_convention = naming_convention
self._const_name = const.name
self.active_convention = active_convention

def _debug_dict(self):
# used to generate a better exception message for debugging
_debug_dict = {"table": "", "columns": []}
try:
_debug_dict["table"] = self.const.table.name
_debug_dict["columns"] = [i.name for i in self.const.columns]
except:
pass
return _debug_dict

def _key_table_name(self):
return self.table.name
Expand All @@ -44,14 +63,49 @@ def _column_X(self, idx):
fk = self.const.elements[idx]
return fk.parent
else:
if not self.const.columns:
_debug_dict = self._debug_dict()
raise exc.InvalidRequestError(
'Naming convention "%s" requires column %s for a '
'constraint, however the constraint does not have that '
'number of columns. Check table "%s" and column(s) "%s".'
% (
self.active_convention,
idx,
_debug_dict.get("table", ""),
_debug_dict.get("columns", ""),
)
)
return list(self.const.columns)[idx]

def _key_constraint_name(self):
if isinstance(self._const_name, (type(None), _defer_none_name)):
_debug_dict = self._debug_dict()
if self.const._type_bound:
raise exc.InvalidRequestError(
'Naming convention "%s" failed to apply to CHECK '
'constraint on table "%s" with column "%s" since the '
'constraint has no name, and the convention uses the '
'"%%(constraint_name)s" token. Consider using the '
'"type_ck" naming convention for type-bound CHECK '
'constraints so that no name is necessary.'
% (
self.active_convention,
_debug_dict.get("table", ""),
_debug_dict.get("columns", ""),
),
code="f0f1",
)
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. "
'Check naming convention "%s", table "%s", '
'and column(s) "%s".'
% (
self.active_convention,
_debug_dict.get("table", ""),
_debug_dict.get("columns", ""),
)
)
if not isinstance(self._const_name, conv):
self.const.name = None
Expand Down Expand Up @@ -86,8 +140,8 @@ def _key_referred_column_X_name(self, idx):
return fk.column.name

def __getitem__(self, key):
if key in self.convention:
return self.convention[key](self.const, self.table)
if key in self.naming_convention:
return self.naming_convention[key](self.const, self.table)
elif hasattr(self, "_key_%s" % key):
return getattr(self, "_key_%s" % key)()
else:
Expand Down Expand Up @@ -118,7 +172,8 @@ def __getitem__(self, key):
raise KeyError(key)


_prefix_dict = {
# NOTE: "base" prefixes might be augmented by `_get_prefixes()`
_base_prefix_dict = {
Index: "ix",
PrimaryKeyConstraint: "pk",
CheckConstraint: "ck",
Expand All @@ -127,11 +182,28 @@ def __getitem__(self, key):
}


def _get_convention(dict_, key):
def _get_prefixes(const):
"""
`_get_prefixes(const)` allows for a `_base_prefix` dict item to be
augmented in certain use-cases.
* `_type_bound` constraints, which are automatically created for Bool/Enum
validation on certain backends will prefer a `type_ck` prefix
"""
for super_ in type(const).__mro__:
if super_ in _base_prefix_dict:
prefix = _base_prefix_dict[super_]
if isinstance(const, Constraint) and const._type_bound:
if isinstance(const.name, _defer_none_name):
# only use the `type_` prefix if no name was presented
yield "type_%s" % prefix, super_
yield prefix, super_

for super_ in key.__mro__:
if super_ in _prefix_dict and _prefix_dict[super_] in dict_:
return dict_[_prefix_dict[super_]]

def _get_convention(dict_, const):
for prefix, super_ in _get_prefixes(const):
if prefix in dict_:
return dict_[prefix]
elif super_ in dict_:
return dict_[super_]
else:
Expand All @@ -140,24 +212,26 @@ def _get_convention(dict_, key):

def _constraint_name_for_table(const, table):
metadata = table.metadata
convention = _get_convention(metadata.naming_convention, type(const))
active_convention = _get_convention(metadata.naming_convention, const)

if isinstance(const.name, conv):
return const.name
elif (
convention is not None
active_convention is not None
and not isinstance(const.name, conv)
and (
const.name is None
or "constraint_name" in convention
or "constraint_name" in active_convention
or isinstance(const.name, _defer_name)
)
):
return conv(
convention
% ConventionDict(const, table, metadata.naming_convention)
active_convention
% ConventionDict(
const, table, metadata.naming_convention, active_convention
)
)
elif isinstance(convention, _defer_none_name):
elif isinstance(active_convention, _defer_none_name):
return None


Expand Down
5 changes: 3 additions & 2 deletions lib/sqlalchemy/sql/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -3995,8 +3995,9 @@ def __init__(
class
* a string mnemonic for one of the known constraint classes;
``"fk"``, ``"pk"``, ``"ix"``, ``"ck"``, ``"uq"`` for foreign key,
primary key, index, check, and unique constraint, respectively.
``"fk"``, ``"pk"``, ``"ix"``, ``"ck"``, ``"type_ck"``, ``"uq"`` for
foreign key, primary key, index, check, type_check, and unique
constraint, respectively.
* the string name of a user-defined "token" that can be used
to define new naming tokens.
Expand Down
Loading

0 comments on commit b7047f9

Please sign in to comment.