]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
- restate sort_tables in terms of a more fine grained
authorMike Bayer <mike_mp@zzzcomputing.com>
Thu, 1 Jan 2015 18:47:08 +0000 (13:47 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Fri, 2 Jan 2015 01:17:06 +0000 (20:17 -0500)
sort_tables_and_constraints function.
- The DDL generation system of :meth:`.MetaData.create_all`
and :meth:`.Metadata.drop_all` has been enhanced to in most
cases automatically handle the case of mutually dependent
foreign key constraints; the need for the
:paramref:`.ForeignKeyConstraint.use_alter` flag is greatly
reduced.  The system also works for constraints which aren't given
a name up front; only in the case of DROP is a name required for
at least one of the constraints involved in the cycle.
fixes #3282

17 files changed:
doc/build/changelog/changelog_10.rst
doc/build/changelog/migration_10.rst
doc/build/core/constraints.rst
doc/build/core/ddl.rst
lib/sqlalchemy/dialects/mysql/base.py
lib/sqlalchemy/dialects/sqlite/base.py
lib/sqlalchemy/engine/reflection.py
lib/sqlalchemy/schema.py
lib/sqlalchemy/sql/compiler.py
lib/sqlalchemy/sql/ddl.py
lib/sqlalchemy/sql/schema.py
lib/sqlalchemy/testing/__init__.py
lib/sqlalchemy/testing/plugin/plugin_base.py
lib/sqlalchemy/testing/util.py
test/orm/test_cycles.py
test/sql/test_constraints.py
test/sql/test_ddlemit.py

index 4b3a17367f946a1a1f7f23cab706f99ad0bd830a..95eaff0f1eaa24cf863d7c465f5fc9b595e0a787 100644 (file)
     series as well.  For changes that are specific to 1.0 with an emphasis
     on compatibility concerns, see :doc:`/changelog/migration_10`.
 
+    .. change::
+        :tags: feature, schema
+        :tickets: 3282
+
+        The DDL generation system of :meth:`.MetaData.create_all`
+        and :meth:`.MetaData.drop_all` has been enhanced to in most
+        cases automatically handle the case of mutually dependent
+        foreign key constraints; the need for the
+        :paramref:`.ForeignKeyConstraint.use_alter` flag is greatly
+        reduced.  The system also works for constraints which aren't given
+        a name up front; only in the case of DROP is a name required for
+        at least one of the constraints involved in the cycle.
+
+        .. seealso::
+
+            :ref:`feature_3282`
+
     .. change::
         :tags: feature, schema
 
index 829d04c512f2a535de7d33af8ea1c7b020fbd415..f9c26017cac54a40c670bd2122d8bef6efacddc4 100644 (file)
@@ -488,6 +488,35 @@ wishes to support the new feature should now call upon the ``._limit_clause``
 and ``._offset_clause`` attributes to receive the full SQL expression, rather
 than the integer value.
 
+.. _feature_3282:
+
+The ``use_alter`` flag on ``ForeignKeyConstraint`` is no longer needed
+----------------------------------------------------------------------
+
+The :meth:`.MetaData.create_all` and :meth:`.MetaData.drop_all` methods will
+now make use of a system that automatically renders an ALTER statement
+for foreign key constraints that are involved in mutually-dependent cycles
+between tables, without the
+need to specify :paramref:`.ForeignKeyConstraint.use_alter`.   Additionally,
+the foreign key constraints no longer need to have a name in order to be
+created via ALTER; only the DROP operation requires a name.   In the case
+of a DROP, the feature will ensure that only constraints which have
+explicit names are actually included as ALTER statements.  In the
+case of an unresolvable cycle within a DROP, the system emits
+a succinct and clear error message now if the DROP cannot proceed.
+
+The :paramref:`.ForeignKeyConstraint.use_alter` and
+:paramref:`.ForeignKey.use_alter` flags remain in place, and continue to have
+the same effect of establishing those constraints for which ALTER is
+required during a CREATE/DROP scenario.
+
+.. seealso::
+
+    :ref:`use_alter` - full description of the new behavior.
+
+
+:ticket:`3282`
+
 .. _change_2051:
 
 .. _feature_insert_from_select_defaults:
index 9bf510d6a3f5582179f62de8abb2364e2c65bf4d..a1130010033c9875c8ab4f79b37298875fe2cec5 100644 (file)
@@ -95,40 +95,179 @@ foreign key referencing two columns.
 Creating/Dropping Foreign Key Constraints via ALTER
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-In all the above examples, the :class:`~sqlalchemy.schema.ForeignKey` object
-causes the "REFERENCES" keyword to be added inline to a column definition
-within a "CREATE TABLE" statement when
-:func:`~sqlalchemy.schema.MetaData.create_all` is issued, and
-:class:`~sqlalchemy.schema.ForeignKeyConstraint` invokes the "CONSTRAINT"
-keyword inline with "CREATE TABLE". There are some cases where this is
-undesirable, particularly when two tables reference each other mutually, each
-with a foreign key referencing the other. In such a situation at least one of
-the foreign key constraints must be generated after both tables have been
-built. To support such a scheme, :class:`~sqlalchemy.schema.ForeignKey` and
-:class:`~sqlalchemy.schema.ForeignKeyConstraint` offer the flag
-``use_alter=True``. When using this flag, the constraint will be generated
-using a definition similar to "ALTER TABLE <tablename> ADD CONSTRAINT <name>
-...". Since a name is required, the ``name`` attribute must also be specified.
-For example::
-
-    node = Table('node', meta,
+The behavior we've seen in tutorials and elsewhere involving
+foreign keys with DDL illustrates that the constraints are typically
+rendered "inline" within the CREATE TABLE statement, such as:
+
+.. sourcecode:: sql
+
+    CREATE TABLE addresses (
+        id INTEGER NOT NULL,
+        user_id INTEGER,
+        email_address VARCHAR NOT NULL,
+        PRIMARY KEY (id),
+        CONSTRAINT user_id_fk FOREIGN KEY(user_id) REFERENCES users (id)
+    )
+
+The ``CONSTRAINT .. FOREIGN KEY`` directive is used to create the constraint
+in an "inline" fashion within the CREATE TABLE definition.   The
+:meth:`.MetaData.create_all` and :meth:`.MetaData.drop_all` methods do
+this by default, using a topological sort of all the :class:`.Table` objects
+involved such that tables are created and dropped in order of their foreign
+key dependency (this sort is also available via the
+:attr:`.MetaData.sorted_tables` accessor).
+
+This approach can't work when two or more foreign key constraints are
+involved in a "dependency cycle", where a set of tables
+are mutually dependent on each other, assuming the backend enforces foreign
+keys (always the case except on SQLite, MySQL/MyISAM).   The methods will
+therefore break out constraints in such a cycle into separate ALTER
+statements, on all backends other than SQLite which does not support
+most forms of ALTER.  Given a schema like::
+
+    node = Table(
+        'node', metadata,
         Column('node_id', Integer, primary_key=True),
-        Column('primary_element', Integer,
-            ForeignKey('element.element_id', use_alter=True, name='fk_node_element_id')
+        Column(
+            'primary_element', Integer,
+            ForeignKey('element.element_id')
         )
     )
 
-    element = Table('element', meta,
+    element = Table(
+        'element', metadata,
         Column('element_id', Integer, primary_key=True),
         Column('parent_node_id', Integer),
         ForeignKeyConstraint(
-            ['parent_node_id'],
-            ['node.node_id'],
-            use_alter=True,
+            ['parent_node_id'], ['node.node_id'],
             name='fk_element_parent_node_id'
         )
     )
 
+When we call upon :meth:`.MetaData.create_all` on a backend such as the
+Postgresql backend, the cycle between these two tables is resolved and the
+constraints are created separately:
+
+.. sourcecode:: pycon+sql
+
+    >>> with engine.connect() as conn:
+    ...    metadata.create_all(conn, checkfirst=False)
+    {opensql}CREATE TABLE element (
+        element_id SERIAL NOT NULL,
+        parent_node_id INTEGER,
+        PRIMARY KEY (element_id)
+    )
+
+    CREATE TABLE node (
+        node_id SERIAL NOT NULL,
+        primary_element INTEGER,
+        PRIMARY KEY (node_id)
+    )
+
+    ALTER TABLE element ADD CONSTRAINT fk_element_parent_node_id
+        FOREIGN KEY(parent_node_id) REFERENCES node (node_id)
+    ALTER TABLE node ADD FOREIGN KEY(primary_element)
+        REFERENCES element (element_id)
+    {stop}
+
+In order to emit DROP for these tables, the same logic applies, however
+note here that in SQL, to emit DROP CONSTRAINT requires that the constraint
+has a name.  In the case of the ``'node'`` table above, we haven't named
+this constraint; the system will therefore attempt to emit DROP for only
+those constraints that are named:
+
+.. NOTE: the parser is doing something wrong with the DROP here,
+   if the "DROP TABLE element" is second, the "t" is being chopped off;
+   it is specific to the letter "t".    Look into this at some point
+
+.. sourcecode:: pycon+sql
+
+    >>> with engine.connect() as conn:
+    ...    metadata.drop_all(conn, checkfirst=False)
+    {opensql}ALTER TABLE element DROP CONSTRAINT fk_element_parent_node_id
+    DROP TABLE element
+    DROP TABLE node
+    {stop}
+
+
+In the case where the cycle cannot be resolved, such as if we hadn't applied
+a name to either constraint here, we will receive the following error::
+
+    sqlalchemy.exc.CircularDependencyError: Can't sort tables for DROP;
+    an unresolvable foreign key dependency exists between tables:
+    element, node.  Please ensure that the ForeignKey and ForeignKeyConstraint
+    objects involved in the cycle have names so that they can be dropped
+    using DROP CONSTRAINT.
+
+This error only applies to the DROP case as we can emit "ADD CONSTRAINT"
+in the CREATE case without a name; the database typically assigns one
+automatically.
+
+The :paramref:`.ForeignKeyConstraint.use_alter` and
+:paramref:`.ForeignKey.use_alter` keyword arguments can be used
+to manually resolve dependency cycles.  We can add this flag only to
+the ``'element'`` table as follows::
+
+    element = Table(
+        'element', metadata,
+        Column('element_id', Integer, primary_key=True),
+        Column('parent_node_id', Integer),
+        ForeignKeyConstraint(
+            ['parent_node_id'], ['node.node_id'],
+            use_alter=True, name='fk_element_parent_node_id'
+        )
+    )
+
+in our CREATE DDL we will see the ALTER statement only for this constraint,
+and not the other one:
+
+.. sourcecode:: pycon+sql
+
+    >>> with engine.connect() as conn:
+    ...    metadata.create_all(conn, checkfirst=False)
+    {opensql}CREATE TABLE element (
+        element_id SERIAL NOT NULL,
+        parent_node_id INTEGER,
+        PRIMARY KEY (element_id)
+    )
+
+    CREATE TABLE node (
+        node_id SERIAL NOT NULL,
+        primary_element INTEGER,
+        PRIMARY KEY (node_id),
+        FOREIGN KEY(primary_element) REFERENCES element (element_id)
+    )
+
+    ALTER TABLE element ADD CONSTRAINT fk_element_parent_node_id
+    FOREIGN KEY(parent_node_id) REFERENCES node (node_id)
+    {stop}
+
+:paramref:`.ForeignKeyConstraint.use_alter` and
+:paramref:`.ForeignKey.use_alter`, when used in conjunction with a drop
+operation, will require that the constraint is named, else an error
+like the following is generated::
+
+    sqlalchemy.exc.CompileError: Can't emit DROP CONSTRAINT for constraint
+    ForeignKeyConstraint(...); it has no name
+
+.. versionchanged:: 1.0.0 - The DDL system invoked by
+   :meth:`.MetaData.create_all`
+   and :meth:`.MetaData.drop_all` will now automatically resolve mutually
+   depdendent foreign keys between tables declared by
+   :class:`.ForeignKeyConstraint` and :class:`.ForeignKey` objects, without
+   the need to explicitly set the :paramref:`.ForeignKeyConstraint.use_alter`
+   flag.
+
+.. versionchanged:: 1.0.0 - The :paramref:`.ForeignKeyConstraint.use_alter`
+   flag can be used with an un-named constraint; only the DROP operation
+   will emit a specific error when actually called upon.
+
+.. seealso::
+
+    :ref:`constraint_naming_conventions`
+
+    :func:`.sort_tables_and_constraints`
+
 .. _on_update_on_delete:
 
 ON UPDATE and ON DELETE
index b8bdd1a20209de07d87e99fb06567015e454f52e..0ba2f280648437796e1087d8a3f7fabef0f6ea44 100644 (file)
@@ -220,6 +220,10 @@ details.
 DDL Expression Constructs API
 -----------------------------
 
+.. autofunction:: sort_tables
+
+.. autofunction:: sort_tables_and_constraints
+
 .. autoclass:: DDLElement
     :members:
     :undoc-members:
index c868f58b2be1a7ba43889598278dd96ce18bcc8f..5f990ea4e7db4e5c4645c0bb42c2022df793d4e5 100644 (file)
@@ -1767,10 +1767,10 @@ class MySQLCompiler(compiler.SQLCompiler):
 #       creation of foreign key constraints fails."
 
 class MySQLDDLCompiler(compiler.DDLCompiler):
-    def create_table_constraints(self, table):
+    def create_table_constraints(self, table, **kw):
         """Get table constraints."""
         constraint_string = super(
-            MySQLDDLCompiler, self).create_table_constraints(table)
+            MySQLDDLCompiler, self).create_table_constraints(table, **kw)
 
         # why self.dialect.name and not 'mysql'?  because of drizzle
         is_innodb = 'engine' in table.dialect_options[self.dialect.name] and \
index e0b2875e8676db9de15d67936a97995a920d399d..3d7b0788b6cc5504a61a959f42882b36dd18f4e8 100644 (file)
@@ -201,6 +201,15 @@ new connections through the usage of events::
         cursor.execute("PRAGMA foreign_keys=ON")
         cursor.close()
 
+.. warning::
+
+    When SQLite foreign keys are enabled, it is **not possible**
+    to emit CREATE or DROP statements for tables that contain
+    mutually-dependent foreign key constraints;
+    to emit the DDL for these tables requires that ALTER TABLE be used to
+    create or drop these constraints separately, for which SQLite has
+    no support.
+
 .. seealso::
 
     `SQLite Foreign Key Support <http://www.sqlite.org/foreignkeys.html>`_
@@ -208,6 +217,9 @@ new connections through the usage of events::
 
     :ref:`event_toplevel` - SQLAlchemy event API.
 
+    :ref:`use_alter` - more information on SQLAlchemy's facilities for handling
+     mutually-dependent foreign key constraints.
+
 .. _sqlite_type_reflection:
 
 Type Reflection
index 25f084c15701e31c1a57ec5e318fd6e329ce95fa..6e102aad6be451eb1b9984704a3203eea7053390 100644 (file)
@@ -173,7 +173,14 @@ class Inspector(object):
          passed as ``None``.  For special quoting, use :class:`.quoted_name`.
 
         :param order_by: Optional, may be the string "foreign_key" to sort
-         the result on foreign key dependencies.
+         the result on foreign key dependencies.  Does not automatically
+         resolve cycles, and will raise :class:`.CircularDependencyError`
+         if cycles exist.
+
+         .. deprecated:: 1.0.0 - see
+            :meth:`.Inspector.get_sorted_table_and_fkc_names` for a version
+            of this which resolves foreign key cycles between tables
+            automatically.
 
          .. versionchanged:: 0.8 the "foreign_key" sorting sorts tables
             in order of dependee to dependent; that is, in creation
@@ -183,6 +190,8 @@ class Inspector(object):
 
         .. seealso::
 
+            :meth:`.Inspector.get_sorted_table_and_fkc_names`
+
             :attr:`.MetaData.sorted_tables`
 
         """
@@ -201,6 +210,64 @@ class Inspector(object):
             tnames = list(topological.sort(tuples, tnames))
         return tnames
 
+    def get_sorted_table_and_fkc_names(self, schema=None):
+        """Return dependency-sorted table and foreign key constraint names in
+        referred to within a particular schema.
+
+        This will yield 2-tuples of
+        ``(tablename, [(tname, fkname), (tname, fkname), ...])``
+        consisting of table names in CREATE order grouped with the foreign key
+        constraint names that are not detected as belonging to a cycle.
+        The final element
+        will be ``(None, [(tname, fkname), (tname, fkname), ..])``
+        which will consist of remaining
+        foreign key constraint names that would require a separate CREATE
+        step after-the-fact, based on dependencies between tables.
+
+        .. versionadded:: 1.0.-
+
+        .. seealso::
+
+            :meth:`.Inspector.get_table_names`
+
+            :func:`.sort_tables_and_constraints` - similar method which works
+             with an already-given :class:`.MetaData`.
+
+        """
+        if hasattr(self.dialect, 'get_table_names'):
+            tnames = self.dialect.get_table_names(
+                self.bind, schema, info_cache=self.info_cache)
+        else:
+            tnames = self.engine.table_names(schema)
+
+        tuples = set()
+        remaining_fkcs = set()
+
+        fknames_for_table = {}
+        for tname in tnames:
+            fkeys = self.get_foreign_keys(tname, schema)
+            fknames_for_table[tname] = set(
+                [fk['name'] for fk in fkeys]
+            )
+            for fkey in fkeys:
+                if tname != fkey['referred_table']:
+                    tuples.add((fkey['referred_table'], tname))
+        try:
+            candidate_sort = list(topological.sort(tuples, tnames))
+        except exc.CircularDependencyError as err:
+            for edge in err.edges:
+                tuples.remove(edge)
+                remaining_fkcs.update(
+                    (edge[1], fkc)
+                    for fkc in fknames_for_table[edge[1]]
+                )
+
+            candidate_sort = list(topological.sort(tuples, tnames))
+        return [
+            (tname, fknames_for_table[tname].difference(remaining_fkcs))
+            for tname in candidate_sort
+        ] + [(None, list(remaining_fkcs))]
+
     def get_temp_table_names(self):
         """return a list of temporary table names for the current bind.
 
index 95ebd05db7a3f54059d2d62235b3d7cd5ef1468e..285ae579fb745941288d57a6e7c0320b497f7e66 100644 (file)
@@ -59,5 +59,7 @@ from .sql.ddl import (
     DDLBase,
     DDLElement,
     _CreateDropBase,
-    _DDLCompiles
+    _DDLCompiles,
+    sort_tables,
+    sort_tables_and_constraints
 )
index 9304bba9ffe51582dab66966d85c7d977a3f7a30..ca14c93710034315ab968043b81ef2f9febdf081 100644 (file)
@@ -2102,7 +2102,9 @@ class DDLCompiler(Compiled):
                         (table.description, column.name, ce.args[0])
                     ))
 
-        const = self.create_table_constraints(table)
+        const = self.create_table_constraints(
+            table, _include_foreign_key_constraints=
+            create.include_foreign_key_constraints)
         if const:
             text += ", \n\t" + const
 
@@ -2126,7 +2128,9 @@ class DDLCompiler(Compiled):
 
         return text
 
-    def create_table_constraints(self, table):
+    def create_table_constraints(
+        self, table,
+            _include_foreign_key_constraints=None):
 
         # On some DB order is significant: visit PK first, then the
         # other constraints (engine.ReflectionTest.testbasic failed on FB2)
@@ -2134,8 +2138,15 @@ class DDLCompiler(Compiled):
         if table.primary_key:
             constraints.append(table.primary_key)
 
+        all_fkcs = table.foreign_key_constraints
+        if _include_foreign_key_constraints is not None:
+            omit_fkcs = all_fkcs.difference(_include_foreign_key_constraints)
+        else:
+            omit_fkcs = set()
+
         constraints.extend([c for c in table._sorted_constraints
-                            if c is not table.primary_key])
+                            if c is not table.primary_key and
+                            c not in omit_fkcs])
 
         return ", \n\t".join(
             p for p in
@@ -2230,9 +2241,19 @@ class DDLCompiler(Compiled):
             self.preparer.format_sequence(drop.element)
 
     def visit_drop_constraint(self, drop):
+        constraint = drop.element
+        if constraint.name is not None:
+            formatted_name = self.preparer.format_constraint(constraint)
+        else:
+            formatted_name = None
+
+        if formatted_name is None:
+            raise exc.CompileError(
+                "Can't emit DROP CONSTRAINT for constraint %r; "
+                "it has no name" % drop.element)
         return "ALTER TABLE %s DROP CONSTRAINT %s%s" % (
             self.preparer.format_table(drop.element.table),
-            self.preparer.format_constraint(drop.element),
+            formatted_name,
             drop.cascade and " CASCADE" or ""
         )
 
index 534322c8d1a2170ef5e8b6fb09c6ee4b54fba1e2..331a283f01c044883a59fe22e037c8e64a9f7dcc 100644 (file)
@@ -12,7 +12,6 @@ to invoke them for a create/drop call.
 
 from .. import util
 from .elements import ClauseElement
-from .visitors import traverse
 from .base import Executable, _generative, SchemaVisitor, _bind_or_error
 from ..util import topological
 from .. import event
@@ -464,19 +463,28 @@ class CreateTable(_CreateDropBase):
 
     __visit_name__ = "create_table"
 
-    def __init__(self, element, on=None, bind=None):
+    def __init__(
+            self, element, on=None, bind=None,
+            include_foreign_key_constraints=None):
         """Create a :class:`.CreateTable` construct.
 
         :param element: a :class:`.Table` that's the subject
          of the CREATE
         :param on: See the description for 'on' in :class:`.DDL`.
         :param bind: See the description for 'bind' in :class:`.DDL`.
+        :param include_foreign_key_constraints: optional sequence of
+         :class:`.ForeignKeyConstraint` objects that will be included
+         inline within the CREATE construct; if omitted, all foreign key
+         constraints that do not specify use_alter=True are included.
+
+         .. versionadded:: 1.0.0
 
         """
         super(CreateTable, self).__init__(element, on=on, bind=bind)
         self.columns = [CreateColumn(column)
                         for column in element.columns
                         ]
+        self.include_foreign_key_constraints = include_foreign_key_constraints
 
 
 class _DropView(_CreateDropBase):
@@ -696,8 +704,10 @@ class SchemaGenerator(DDLBase):
             tables = self.tables
         else:
             tables = list(metadata.tables.values())
-        collection = [t for t in sort_tables(tables)
-                      if self._can_create_table(t)]
+
+        collection = sort_tables_and_constraints(
+            [t for t in tables if self._can_create_table(t)])
+
         seq_coll = [s for s in metadata._sequences.values()
                     if s.column is None and self._can_create_sequence(s)]
 
@@ -709,15 +719,23 @@ class SchemaGenerator(DDLBase):
         for seq in seq_coll:
             self.traverse_single(seq, create_ok=True)
 
-        for table in collection:
-            self.traverse_single(table, create_ok=True)
+        for table, fkcs in collection:
+            if table is not None:
+                self.traverse_single(
+                    table, create_ok=True,
+                    include_foreign_key_constraints=fkcs)
+            else:
+                for fkc in fkcs:
+                    self.traverse_single(fkc)
 
         metadata.dispatch.after_create(metadata, self.connection,
                                        tables=collection,
                                        checkfirst=self.checkfirst,
                                        _ddl_runner=self)
 
-    def visit_table(self, table, create_ok=False):
+    def visit_table(
+            self, table, create_ok=False,
+            include_foreign_key_constraints=None):
         if not create_ok and not self._can_create_table(table):
             return
 
@@ -729,7 +747,15 @@ class SchemaGenerator(DDLBase):
             if column.default is not None:
                 self.traverse_single(column.default)
 
-        self.connection.execute(CreateTable(table))
+        if not self.dialect.supports_alter:
+            # e.g., don't omit any foreign key constraints
+            include_foreign_key_constraints = None
+
+        self.connection.execute(
+            CreateTable(
+                table,
+                include_foreign_key_constraints=include_foreign_key_constraints
+            ))
 
         if hasattr(table, 'indexes'):
             for index in table.indexes:
@@ -739,6 +765,11 @@ class SchemaGenerator(DDLBase):
                                     checkfirst=self.checkfirst,
                                     _ddl_runner=self)
 
+    def visit_foreign_key_constraint(self, constraint):
+        if not self.dialect.supports_alter:
+            return
+        self.connection.execute(AddConstraint(constraint))
+
     def visit_sequence(self, sequence, create_ok=False):
         if not create_ok and not self._can_create_sequence(sequence):
             return
@@ -765,11 +796,33 @@ class SchemaDropper(DDLBase):
         else:
             tables = list(metadata.tables.values())
 
-        collection = [
-            t
-            for t in reversed(sort_tables(tables))
-            if self._can_drop_table(t)
-        ]
+        try:
+            collection = reversed(
+                sort_tables_and_constraints(
+                    [t for t in tables if self._can_drop_table(t)],
+                    filter_fn=
+                    lambda constraint: True if not self.dialect.supports_alter
+                    else False if constraint.name is None
+                    else None
+                )
+            )
+        except exc.CircularDependencyError as err2:
+            util.raise_from_cause(
+                exc.CircularDependencyError(
+                    err2.message,
+                    err2.cycles, err2.edges,
+                    msg="Can't sort tables for DROP; an "
+                    "unresolvable foreign key "
+                    "dependency exists between tables: %s.  Please ensure "
+                    "that the ForeignKey and ForeignKeyConstraint objects "
+                    "involved in the cycle have "
+                    "names so that they can be dropped using DROP CONSTRAINT."
+                    % (
+                        ", ".join(sorted([t.fullname for t in err2.cycles]))
+                    )
+
+                )
+            )
 
         seq_coll = [
             s
@@ -781,8 +834,13 @@ class SchemaDropper(DDLBase):
             metadata, self.connection, tables=collection,
             checkfirst=self.checkfirst, _ddl_runner=self)
 
-        for table in collection:
-            self.traverse_single(table, drop_ok=True)
+        for table, fkcs in collection:
+            if table is not None:
+                self.traverse_single(
+                    table, drop_ok=True)
+            else:
+                for fkc in fkcs:
+                    self.traverse_single(fkc)
 
         for seq in seq_coll:
             self.traverse_single(seq, drop_ok=True)
@@ -830,6 +888,11 @@ class SchemaDropper(DDLBase):
                                   checkfirst=self.checkfirst,
                                   _ddl_runner=self)
 
+    def visit_foreign_key_constraint(self, constraint):
+        if not self.dialect.supports_alter:
+            return
+        self.connection.execute(DropConstraint(constraint))
+
     def visit_sequence(self, sequence, drop_ok=False):
         if not drop_ok and not self._can_drop_sequence(sequence):
             return
@@ -837,32 +900,159 @@ class SchemaDropper(DDLBase):
 
 
 def sort_tables(tables, skip_fn=None, extra_dependencies=None):
-    """sort a collection of Table objects in order of
-                their foreign-key dependency."""
+    """sort a collection of :class:`.Table` objects based on dependency.
 
-    tables = list(tables)
-    tuples = []
-    if extra_dependencies is not None:
-        tuples.extend(extra_dependencies)
+    This is a dependency-ordered sort which will emit :class:`.Table`
+    objects such that they will follow their dependent :class:`.Table` objects.
+    Tables are dependent on another based on the presence of
+    :class:`.ForeignKeyConstraint` objects as well as explicit dependencies
+    added by :meth:`.Table.add_is_dependent_on`.
+
+    .. warning::
+
+        The :func:`.sort_tables` function cannot by itself accommodate
+        automatic resolution of dependency cycles between tables, which
+        are usually caused by mutually dependent foreign key constraints.
+        To resolve these cycles, either the
+        :paramref:`.ForeignKeyConstraint.use_alter` parameter may be appled
+        to those constraints, or use the
+        :func:`.sql.sort_tables_and_constraints` function which will break
+        out foreign key constraints involved in cycles separately.
+
+    :param tables: a sequence of :class:`.Table` objects.
+
+    :param skip_fn: optional callable which will be passed a
+     :class:`.ForeignKey` object; if it returns True, this
+     constraint will not be considered as a dependency.  Note this is
+     **different** from the same parameter in
+     :func:`.sort_tables_and_constraints`, which is
+     instead passed the owning :class:`.ForeignKeyConstraint` object.
+
+    :param extra_dependencies: a sequence of 2-tuples of tables which will
+     also be considered as dependent on each other.
+
+    .. seealso::
+
+        :func:`.sort_tables_and_constraints`
+
+        :meth:`.MetaData.sorted_tables` - uses this function to sort
+
+
+    """
+
+    if skip_fn is not None:
+        def _skip_fn(fkc):
+            for fk in fkc.elements:
+                if skip_fn(fk):
+                    return True
+            else:
+                return None
+    else:
+        _skip_fn = None
+
+    return [
+        t for (t, fkcs) in
+        sort_tables_and_constraints(
+            tables, filter_fn=_skip_fn, extra_dependencies=extra_dependencies)
+        if t is not None
+    ]
+
+
+def sort_tables_and_constraints(
+        tables, filter_fn=None, extra_dependencies=None):
+    """sort a collection of :class:`.Table`  / :class:`.ForeignKeyConstraint`
+    objects.
+
+    This is a dependency-ordered sort which will emit tuples of
+    ``(Table, [ForeignKeyConstraint, ...])`` such that each
+    :class:`.Table` follows its dependent :class:`.Table` objects.
+    Remaining :class:`.ForeignKeyConstraint` objects that are separate due to
+    dependency rules not satisifed by the sort are emitted afterwards
+    as ``(None, [ForeignKeyConstraint ...])``.
+
+    Tables are dependent on another based on the presence of
+    :class:`.ForeignKeyConstraint` objects, explicit dependencies
+    added by :meth:`.Table.add_is_dependent_on`, as well as dependencies
+    stated here using the :paramref:`~.sort_tables_and_constraints.skip_fn`
+    and/or :paramref:`~.sort_tables_and_constraints.extra_dependencies`
+    parameters.
+
+    :param tables: a sequence of :class:`.Table` objects.
+
+    :param filter_fn: optional callable which will be passed a
+     :class:`.ForeignKeyConstraint` object, and returns a value based on
+     whether this constraint should definitely be included or excluded as
+     an inline constraint, or neither.   If it returns False, the constraint
+     will definitely be included as a dependency that cannot be subject
+     to ALTER; if True, it will **only** be included as an ALTER result at
+     the end.   Returning None means the constraint is included in the
+     table-based result unless it is detected as part of a dependency cycle.
+
+    :param extra_dependencies: a sequence of 2-tuples of tables which will
+     also be considered as dependent on each other.
+
+    .. versionadded:: 1.0.0
+
+    .. seealso::
+
+        :func:`.sort_tables`
 
-    def visit_foreign_key(fkey):
-        if fkey.use_alter:
-            return
-        elif skip_fn and skip_fn(fkey):
-            return
-        parent_table = fkey.column.table
-        if parent_table in tables:
-            child_table = fkey.parent.table
-            if parent_table is not child_table:
-                tuples.append((parent_table, child_table))
 
+    """
+
+    fixed_dependencies = set()
+    mutable_dependencies = set()
+
+    if extra_dependencies is not None:
+        fixed_dependencies.update(extra_dependencies)
+
+    remaining_fkcs = set()
     for table in tables:
-        traverse(table,
-                 {'schema_visitor': True},
-                 {'foreign_key': visit_foreign_key})
+        for fkc in table.foreign_key_constraints:
+            if fkc.use_alter is True:
+                remaining_fkcs.add(fkc)
+                continue
+
+            if filter_fn:
+                filtered = filter_fn(fkc)
+
+                if filtered is True:
+                    remaining_fkcs.add(fkc)
+                    continue
 
-        tuples.extend(
-            [parent, table] for parent in table._extra_dependencies
+            dependent_on = fkc.referred_table
+            if dependent_on is not table:
+                mutable_dependencies.add((dependent_on, table))
+
+        fixed_dependencies.update(
+            (parent, table) for parent in table._extra_dependencies
+        )
+
+    try:
+        candidate_sort = list(
+            topological.sort(
+                fixed_dependencies.union(mutable_dependencies), tables
+            )
+        )
+    except exc.CircularDependencyError as err:
+        for edge in err.edges:
+            if edge in mutable_dependencies:
+                table = edge[1]
+                can_remove = [
+                    fkc for fkc in table.foreign_key_constraints
+                    if filter_fn is None or filter_fn(fkc) is not False]
+                remaining_fkcs.update(can_remove)
+                for fkc in can_remove:
+                    dependent_on = fkc.referred_table
+                    if dependent_on is not table:
+                        mutable_dependencies.discard((dependent_on, table))
+        candidate_sort = list(
+            topological.sort(
+                fixed_dependencies.union(mutable_dependencies), tables
+            )
         )
 
-    return list(topological.sort(tuples, tables))
+    return [
+        (table, table.foreign_key_constraints.difference(remaining_fkcs))
+        for table in candidate_sort
+    ] + [(None, list(remaining_fkcs))]
index 71a0c2780a814d11205c86c7eef536fc1db85df6..65a1da8776cb1e70b994257a0c36cdbf206bef6c 100644 (file)
@@ -1476,7 +1476,14 @@ class ForeignKey(DialectKWArgs, SchemaItem):
         :param use_alter: passed to the underlying
             :class:`.ForeignKeyConstraint` to indicate the constraint should
             be generated/dropped externally from the CREATE TABLE/ DROP TABLE
-            statement. See that classes' constructor for details.
+            statement.  See :paramref:`.ForeignKeyConstraint.use_alter`
+            for further description.
+
+            .. seealso::
+
+                :paramref:`.ForeignKeyConstraint.use_alter`
+
+                :ref:`use_alter`
 
         :param match: Optional string. If set, emit MATCH <value> when issuing
             DDL for this constraint. Typical values include SIMPLE, PARTIAL
@@ -2566,11 +2573,23 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
           part of the CREATE TABLE definition. Instead, generate it via an
           ALTER TABLE statement issued after the full collection of tables
           have been created, and drop it via an ALTER TABLE statement before
-          the full collection of tables are dropped. This is shorthand for the
-          usage of :class:`.AddConstraint` and :class:`.DropConstraint`
-          applied as "after-create" and "before-drop" events on the MetaData
-          object.  This is normally used to generate/drop constraints on
-          objects that are mutually dependent on each other.
+          the full collection of tables are dropped.
+
+          The use of :paramref:`.ForeignKeyConstraint.use_alter` is
+          particularly geared towards the case where two or more tables
+          are established within a mutually-dependent foreign key constraint
+          relationship; however, the :meth:`.MetaData.create_all` and
+          :meth:`.MetaData.drop_all` methods will perform this resolution
+          automatically, so the flag is normally not needed.
+
+          .. versionchanged:: 1.0.0  Automatic resolution of foreign key
+             cycles has been added, removing the need to use the
+             :paramref:`.ForeignKeyConstraint.use_alter` in typical use
+             cases.
+
+          .. seealso::
+
+                :ref:`use_alter`
 
         :param match: Optional string. If set, emit MATCH <value> when issuing
           DDL for this constraint. Typical values include SIMPLE, PARTIAL
@@ -2596,8 +2615,6 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
         self.onupdate = onupdate
         self.ondelete = ondelete
         self.link_to_name = link_to_name
-        if self.name is None and use_alter:
-            raise exc.ArgumentError("Alterable Constraint requires a name")
         self.use_alter = use_alter
         self.match = match
 
@@ -2648,7 +2665,7 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
     @property
     def referred_table(self):
         """The :class:`.Table` object to which this
-        :class:`.ForeignKeyConstraint references.
+        :class:`.ForeignKeyConstraint` references.
 
         This is a dynamically calculated attribute which may not be available
         if the constraint and/or parent table is not yet associated with
@@ -2716,15 +2733,6 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
 
         self._validate_dest_table(table)
 
-        if self.use_alter:
-            def supports_alter(ddl, event, schema_item, bind, **kw):
-                return table in set(kw['tables']) and \
-                    bind.dialect.supports_alter
-
-            event.listen(table.metadata, "after_create",
-                         ddl.AddConstraint(self, on=supports_alter))
-            event.listen(table.metadata, "before_drop",
-                         ddl.DropConstraint(self, on=supports_alter))
 
     def copy(self, schema=None, target_table=None, **kw):
         fkc = ForeignKeyConstraint(
@@ -3368,12 +3376,30 @@ class MetaData(SchemaItem):
         order in which they can be created.   To get the order in which
         the tables would be dropped, use the ``reversed()`` Python built-in.
 
+        .. warning::
+
+            The :attr:`.sorted_tables` accessor cannot by itself accommodate
+            automatic resolution of dependency cycles between tables, which
+            are usually caused by mutually dependent foreign key constraints.
+            To resolve these cycles, either the
+            :paramref:`.ForeignKeyConstraint.use_alter` parameter may be appled
+            to those constraints, or use the
+            :func:`.schema.sort_tables_and_constraints` function which will break
+            out foreign key constraints involved in cycles separately.
+
         .. seealso::
 
+            :func:`.schema.sort_tables`
+
+            :func:`.schema.sort_tables_and_constraints`
+
             :attr:`.MetaData.tables`
 
             :meth:`.Inspector.get_table_names`
 
+            :meth:`.Inspector.get_sorted_table_and_fkc_names`
+
+
         """
         return ddl.sort_tables(self.tables.values())
 
index 1f37b4b451a727ed25ea21ce06b7f88ff3ae6708..2375a13a9414e22327915172823a6476af1ee71b 100644 (file)
@@ -23,7 +23,8 @@ from .assertions import emits_warning, emits_warning_on, uses_deprecated, \
     assert_raises_message, AssertsCompiledSQL, ComparesTables, \
     AssertsExecutionResults, expect_deprecated, expect_warnings
 
-from .util import run_as_contextmanager, rowset, fail, provide_metadata, adict
+from .util import run_as_contextmanager, rowset, fail, \
+    provide_metadata, adict, force_drop_names
 
 crashes = skip
 
index 614a12133e3f4eae652108ef298015f39351ad1f..646e4dea25e805862286c5f306dcb1f2b1bcd10c 100644 (file)
@@ -325,19 +325,11 @@ def _prep_testing_database(options, file_config):
                                          schema="test_schema")
                         ))
 
-            for tname in reversed(inspector.get_table_names(
-                    order_by="foreign_key")):
-                e.execute(schema.DropTable(
-                    schema.Table(tname, schema.MetaData())
-                ))
+            util.drop_all_tables(e, inspector)
 
             if config.requirements.schemas.enabled_for_config(cfg):
-                for tname in reversed(inspector.get_table_names(
-                        order_by="foreign_key", schema="test_schema")):
-                    e.execute(schema.DropTable(
-                        schema.Table(tname, schema.MetaData(),
-                                     schema="test_schema")
-                    ))
+                util.drop_all_tables(e, inspector, schema=cfg.test_schema)
+                util.drop_all_tables(e, inspector, schema=cfg.test_schema_2)
 
             if against(cfg, "postgresql"):
                 from sqlalchemy.dialects import postgresql
index 7b3f721a61bc96f9f875ab74eb01bfd54c5efffd..eea39b1f70370e88a2abd720fd17b9c8b25d189c 100644 (file)
@@ -194,6 +194,25 @@ def provide_metadata(fn, *args, **kw):
         self.metadata = prev_meta
 
 
+def force_drop_names(*names):
+    """Force the given table names to be dropped after test complete,
+    isolating for foreign key cycles
+
+    """
+    from . import config
+    from sqlalchemy import inspect
+
+    @decorator
+    def go(fn, *args, **kw):
+
+        try:
+            return fn(*args, **kw)
+        finally:
+            drop_all_tables(
+                config.db, inspect(config.db), include_names=names)
+    return go
+
+
 class adict(dict):
     """Dict keys available as attributes.  Shadows."""
 
@@ -207,3 +226,39 @@ class adict(dict):
         return tuple([self[key] for key in keys])
 
     get_all = __call__
+
+
+def drop_all_tables(engine, inspector, schema=None, include_names=None):
+    from sqlalchemy import Column, Table, Integer, MetaData, \
+        ForeignKeyConstraint
+    from sqlalchemy.schema import DropTable, DropConstraint
+
+    if include_names is not None:
+        include_names = set(include_names)
+
+    with engine.connect() as conn:
+        for tname, fkcs in reversed(
+                inspector.get_sorted_table_and_fkc_names(schema=schema)):
+            if tname:
+                if include_names is not None and tname not in include_names:
+                    continue
+                conn.execute(DropTable(
+                    Table(tname, MetaData())
+                ))
+            elif fkcs:
+                if not engine.dialect.supports_alter:
+                    continue
+                for tname, fkc in fkcs:
+                    if include_names is not None and \
+                            tname not in include_names:
+                        continue
+                    tb = Table(
+                        tname, MetaData(),
+                        Column('x', Integer),
+                        Column('y', Integer),
+                        schema=schema
+                    )
+                    conn.execute(DropConstraint(
+                        ForeignKeyConstraint(
+                            [tb.c.x], [tb.c.y], name=fkc)
+                    ))
index 8e086ff88fbb7faf2fe8ff0cf8d41cef366e877e..fc7059dcb127a23405d7c6d0e5ce9d6eb6858a48 100644 (file)
@@ -284,7 +284,7 @@ class InheritTestTwo(fixtures.MappedTest):
         Table('c', metadata,
             Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
             Column('aid', Integer,
-                   ForeignKey('a.id', use_alter=True, name="foo")))
+                   ForeignKey('a.id', name="foo")))
 
     @classmethod
     def setup_classes(cls):
@@ -334,7 +334,7 @@ class BiDirectionalManyToOneTest(fixtures.MappedTest):
             Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
             Column('data', String(30)),
             Column('t1id', Integer,
-                   ForeignKey('t1.id', use_alter=True, name="foo_fk")))
+                   ForeignKey('t1.id', name="foo_fk")))
         Table('t3', metadata,
             Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
             Column('data', String(30)),
@@ -436,7 +436,7 @@ class BiDirectionalOneToManyTest(fixtures.MappedTest):
         Table('t2', metadata,
               Column('c1', Integer, primary_key=True, test_needs_autoincrement=True),
               Column('c2', Integer,
-                     ForeignKey('t1.c1', use_alter=True, name='t1c1_fk')))
+                     ForeignKey('t1.c1', name='t1c1_fk')))
 
     @classmethod
     def setup_classes(cls):
@@ -491,7 +491,7 @@ class BiDirectionalOneToManyTest2(fixtures.MappedTest):
         Table('t2', metadata,
               Column('c1', Integer, primary_key=True, test_needs_autoincrement=True),
               Column('c2', Integer,
-                     ForeignKey('t1.c1', use_alter=True, name='t1c1_fq')),
+                     ForeignKey('t1.c1', name='t1c1_fq')),
               test_needs_autoincrement=True)
 
         Table('t1_data', metadata,
@@ -572,7 +572,7 @@ class OneToManyManyToOneTest(fixtures.MappedTest):
         Table('ball', metadata,
               Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
               Column('person_id', Integer,
-                     ForeignKey('person.id', use_alter=True, name='fk_person_id')),
+                     ForeignKey('person.id', name='fk_person_id')),
               Column('data', String(30)))
 
         Table('person', metadata,
@@ -1024,7 +1024,7 @@ class SelfReferentialPostUpdateTest3(fixtures.MappedTest):
                      test_needs_autoincrement=True),
               Column('name', String(50), nullable=False),
               Column('child_id', Integer,
-                     ForeignKey('child.id', use_alter=True, name='c1'), nullable=True))
+                     ForeignKey('child.id', name='c1'), nullable=True))
 
         Table('child', metadata,
            Column('id', Integer, primary_key=True,
@@ -1094,11 +1094,11 @@ class PostUpdateBatchingTest(fixtures.MappedTest):
                      test_needs_autoincrement=True),
               Column('name', String(50), nullable=False),
               Column('c1_id', Integer,
-                     ForeignKey('child1.id', use_alter=True, name='c1'), nullable=True),
+                     ForeignKey('child1.id', name='c1'), nullable=True),
               Column('c2_id', Integer,
-                    ForeignKey('child2.id', use_alter=True, name='c2'), nullable=True),
+                    ForeignKey('child2.id', name='c2'), nullable=True),
               Column('c3_id', Integer,
-                       ForeignKey('child3.id', use_alter=True, name='c3'), nullable=True)
+                       ForeignKey('child3.id', name='c3'), nullable=True)
             )
 
         Table('child1', metadata,
index c0b5806ac02a9ee27d986715284520355116775b..604b5efebd499388acf865c3a585b26158f7fc7e 100644 (file)
@@ -58,8 +58,77 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
                         )
         )
 
+    @testing.force_drop_names('a', 'b')
+    def test_fk_cant_drop_cycled_unnamed(self):
+        metadata = MetaData()
+
+        Table("a", metadata,
+              Column('id', Integer, primary_key=True),
+              Column('bid', Integer),
+              ForeignKeyConstraint(["bid"], ["b.id"])
+              )
+        Table(
+            "b", metadata,
+            Column('id', Integer, primary_key=True),
+            Column("aid", Integer),
+            ForeignKeyConstraint(["aid"], ["a.id"]))
+        metadata.create_all(testing.db)
+        if testing.db.dialect.supports_alter:
+            assert_raises_message(
+                exc.CircularDependencyError,
+                "Can't sort tables for DROP; an unresolvable foreign key "
+                "dependency exists between tables: a, b.  Please ensure "
+                "that the ForeignKey and ForeignKeyConstraint objects "
+                "involved in the cycle have names so that they can be "
+                "dropped using DROP CONSTRAINT.",
+                metadata.drop_all, testing.db
+            )
+        else:
+
+            with self.sql_execution_asserter() as asserter:
+                metadata.drop_all(testing.db, checkfirst=False)
+
+            asserter.assert_(
+                AllOf(
+                    CompiledSQL("DROP TABLE a"),
+                    CompiledSQL("DROP TABLE b")
+                )
+            )
+
+    @testing.provide_metadata
+    def test_fk_table_auto_alter_constraint_create(self):
+        metadata = self.metadata
+
+        Table("a", metadata,
+              Column('id', Integer, primary_key=True),
+              Column('bid', Integer),
+              ForeignKeyConstraint(["bid"], ["b.id"])
+              )
+        Table(
+            "b", metadata,
+            Column('id', Integer, primary_key=True),
+            Column("aid", Integer),
+            ForeignKeyConstraint(["aid"], ["a.id"], name="bfk"))
+        self._assert_cyclic_constraint(metadata, auto=True)
+
+    @testing.provide_metadata
+    def test_fk_column_auto_alter_constraint_create(self):
+        metadata = self.metadata
+
+        Table("a", metadata,
+              Column('id', Integer, primary_key=True),
+              Column('bid', Integer, ForeignKey("b.id")),
+              )
+        Table("b", metadata,
+              Column('id', Integer, primary_key=True),
+              Column("aid", Integer,
+                     ForeignKey("a.id", name="bfk")
+                     ),
+              )
+        self._assert_cyclic_constraint(metadata, auto=True)
+
     @testing.provide_metadata
-    def test_cyclic_fk_table_constraint_create(self):
+    def test_fk_table_use_alter_constraint_create(self):
         metadata = self.metadata
 
         Table("a", metadata,
@@ -75,7 +144,7 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
         self._assert_cyclic_constraint(metadata)
 
     @testing.provide_metadata
-    def test_cyclic_fk_column_constraint_create(self):
+    def test_fk_column_use_alter_constraint_create(self):
         metadata = self.metadata
 
         Table("a", metadata,
@@ -90,45 +159,104 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
               )
         self._assert_cyclic_constraint(metadata)
 
-    def _assert_cyclic_constraint(self, metadata):
-        assertions = [
-            CompiledSQL('CREATE TABLE b ('
+    def _assert_cyclic_constraint(self, metadata, auto=False):
+        table_assertions = []
+        if auto:
+            if testing.db.dialect.supports_alter:
+                table_assertions.append(
+                    CompiledSQL('CREATE TABLE b ('
+                                'id INTEGER NOT NULL, '
+                                'aid INTEGER, '
+                                'PRIMARY KEY (id)'
+                                ')'
+                                )
+                )
+            else:
+                table_assertions.append(
+                    CompiledSQL(
+                        'CREATE TABLE b ('
                         'id INTEGER NOT NULL, '
                         'aid INTEGER, '
+                        'PRIMARY KEY (id), '
+                        'CONSTRAINT bfk FOREIGN KEY(aid) REFERENCES a (id)'
+                        ')'
+                    )
+                )
+
+            if testing.db.dialect.supports_alter:
+                table_assertions.append(
+                    CompiledSQL(
+                        'CREATE TABLE a ('
+                        'id INTEGER NOT NULL, '
+                        'bid INTEGER, '
                         'PRIMARY KEY (id)'
                         ')'
-                        ),
-            CompiledSQL('CREATE TABLE a ('
+                    )
+                )
+            else:
+                table_assertions.append(
+                    CompiledSQL(
+                        'CREATE TABLE a ('
                         'id INTEGER NOT NULL, '
                         'bid INTEGER, '
                         'PRIMARY KEY (id), '
                         'FOREIGN KEY(bid) REFERENCES b (id)'
                         ')'
-                        ),
-        ]
+                    )
+                )
+        else:
+            table_assertions.append(
+                CompiledSQL('CREATE TABLE b ('
+                            'id INTEGER NOT NULL, '
+                            'aid INTEGER, '
+                            'PRIMARY KEY (id)'
+                            ')'
+                            )
+            )
+            table_assertions.append(
+                CompiledSQL(
+                    'CREATE TABLE a ('
+                    'id INTEGER NOT NULL, '
+                    'bid INTEGER, '
+                    'PRIMARY KEY (id), '
+                    'FOREIGN KEY(bid) REFERENCES b (id)'
+                    ')'
+                )
+            )
+
+        assertions = [AllOf(*table_assertions)]
         if testing.db.dialect.supports_alter:
-            assertions.append(
+            fk_assertions = []
+            fk_assertions.append(
                 CompiledSQL('ALTER TABLE b ADD CONSTRAINT bfk '
                             'FOREIGN KEY(aid) REFERENCES a (id)')
             )
-        self.assert_sql_execution(
-            testing.db,
-            lambda: metadata.create_all(checkfirst=False),
-            *assertions
-        )
+            if auto:
+                fk_assertions.append(
+                    CompiledSQL('ALTER TABLE a ADD '
+                                'FOREIGN KEY(bid) REFERENCES b (id)')
+                )
+            assertions.append(AllOf(*fk_assertions))
+
+        with self.sql_execution_asserter() as asserter:
+            metadata.create_all(checkfirst=False)
+        asserter.assert_(*assertions)
 
-        assertions = []
         if testing.db.dialect.supports_alter:
-            assertions.append(CompiledSQL('ALTER TABLE b DROP CONSTRAINT bfk'))
-        assertions.extend([
-            CompiledSQL("DROP TABLE a"),
-            CompiledSQL("DROP TABLE b"),
-        ])
-        self.assert_sql_execution(
-            testing.db,
-            lambda: metadata.drop_all(checkfirst=False),
-            *assertions
-        )
+            assertions = [
+                CompiledSQL('ALTER TABLE b DROP CONSTRAINT bfk'),
+                CompiledSQL("DROP TABLE a"),
+                CompiledSQL("DROP TABLE b")
+            ]
+        else:
+            assertions = [AllOf(
+                CompiledSQL("DROP TABLE a"),
+                CompiledSQL("DROP TABLE b")
+            )]
+
+        with self.sql_execution_asserter() as asserter:
+            metadata.drop_all(checkfirst=False),
+        asserter.assert_(*assertions)
 
     @testing.requires.check_constraints
     @testing.provide_metadata
@@ -542,6 +670,33 @@ class ConstraintCompilationTest(fixtures.TestBase, AssertsCompiledSQL):
             "REFERENCES tbl (a) MATCH SIMPLE"
         )
 
+    def test_create_table_omit_fks(self):
+        fkcs = [
+            ForeignKeyConstraint(['a'], ['remote.id'], name='foo'),
+            ForeignKeyConstraint(['b'], ['remote.id'], name='bar'),
+            ForeignKeyConstraint(['c'], ['remote.id'], name='bat'),
+        ]
+        m = MetaData()
+        t = Table(
+            't', m,
+            Column('a', Integer),
+            Column('b', Integer),
+            Column('c', Integer),
+            *fkcs
+        )
+        Table('remote', m, Column('id', Integer, primary_key=True))
+
+        self.assert_compile(
+            schema.CreateTable(t, include_foreign_key_constraints=[]),
+            "CREATE TABLE t (a INTEGER, b INTEGER, c INTEGER)"
+        )
+        self.assert_compile(
+            schema.CreateTable(t, include_foreign_key_constraints=fkcs[0:2]),
+            "CREATE TABLE t (a INTEGER, b INTEGER, c INTEGER, "
+            "CONSTRAINT foo FOREIGN KEY(a) REFERENCES remote (id), "
+            "CONSTRAINT bar FOREIGN KEY(b) REFERENCES remote (id))"
+        )
+
     def test_deferrable_unique(self):
         factory = lambda **kw: UniqueConstraint('b', **kw)
         self._test_deferrable(factory)
index 825f8228ba3051a7b96747a6b5f21503c2205f35..e191beed3ae1c67b7c9d607516ceea1e982be3f6 100644 (file)
@@ -1,6 +1,6 @@
 from sqlalchemy.testing import fixtures
 from sqlalchemy.sql.ddl import SchemaGenerator, SchemaDropper
-from sqlalchemy import MetaData, Table, Column, Integer, Sequence
+from sqlalchemy import MetaData, Table, Column, Integer, Sequence, ForeignKey
 from sqlalchemy import schema
 from sqlalchemy.testing.mock import Mock
 
@@ -42,6 +42,31 @@ class EmitDDLTest(fixtures.TestBase):
             for i in range(1, 6)
         )
 
+    def _use_alter_fixture_one(self):
+        m = MetaData()
+
+        t1 = Table(
+            't1', m, Column('id', Integer, primary_key=True),
+            Column('t2id', Integer, ForeignKey('t2.id'))
+        )
+        t2 = Table(
+            't2', m, Column('id', Integer, primary_key=True),
+            Column('t1id', Integer, ForeignKey('t1.id'))
+        )
+        return m, t1, t2
+
+    def _fk_fixture_one(self):
+        m = MetaData()
+
+        t1 = Table(
+            't1', m, Column('id', Integer, primary_key=True),
+            Column('t2id', Integer, ForeignKey('t2.id'))
+        )
+        t2 = Table(
+            't2', m, Column('id', Integer, primary_key=True),
+        )
+        return m, t1, t2
+
     def _table_seq_fixture(self):
         m = MetaData()
 
@@ -172,6 +197,32 @@ class EmitDDLTest(fixtures.TestBase):
 
         self._assert_drop_tables([t1, t2, t3, t4, t5], generator, m)
 
+    def test_create_metadata_auto_alter_fk(self):
+        m, t1, t2 = self._use_alter_fixture_one()
+        generator = self._mock_create_fixture(
+            False, [t1, t2]
+        )
+        self._assert_create_w_alter(
+            [t1, t2] +
+            list(t1.foreign_key_constraints) +
+            list(t2.foreign_key_constraints),
+            generator,
+            m
+        )
+
+    def test_create_metadata_inline_fk(self):
+        m, t1, t2 = self._fk_fixture_one()
+        generator = self._mock_create_fixture(
+            False, [t1, t2]
+        )
+        self._assert_create_w_alter(
+            [t1, t2] +
+            list(t1.foreign_key_constraints) +
+            list(t2.foreign_key_constraints),
+            generator,
+            m
+        )
+
     def _assert_create_tables(self, elements, generator, argument):
         self._assert_ddl(schema.CreateTable, elements, generator, argument)
 
@@ -188,6 +239,16 @@ class EmitDDLTest(fixtures.TestBase):
             (schema.DropTable, schema.DropSequence),
             elements, generator, argument)
 
+    def _assert_create_w_alter(self, elements, generator, argument):
+        self._assert_ddl(
+            (schema.CreateTable, schema.CreateSequence, schema.AddConstraint),
+            elements, generator, argument)
+
+    def _assert_drop_w_alter(self, elements, generator, argument):
+        self._assert_ddl(
+            (schema.DropTable, schema.DropSequence, schema.DropConstraint),
+            elements, generator, argument)
+
     def _assert_ddl(self, ddl_cls, elements, generator, argument):
         generator.traverse_single(argument)
         for call_ in generator.connection.execute.mock_calls:
@@ -196,4 +257,8 @@ class EmitDDLTest(fixtures.TestBase):
             assert c.element in elements, "element %r was not expected"\
                 % c.element
             elements.remove(c.element)
+            if getattr(c, 'include_foreign_key_constraints', None) is not None:
+                elements[:] = [
+                    e for e in elements
+                    if e not in set(c.include_foreign_key_constraints)]
         assert not elements, "elements remain in list: %r" % elements