summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/build/changelog/changelog_10.rst36
-rw-r--r--doc/build/changelog/migration_10.rst14
-rw-r--r--lib/sqlalchemy/sql/ddl.py46
-rw-r--r--lib/sqlalchemy/testing/assertsql.py10
-rw-r--r--lib/sqlalchemy/testing/engines.py9
-rw-r--r--lib/sqlalchemy/testing/util.py3
-rw-r--r--test/sql/test_constraints.py354
7 files changed, 374 insertions, 98 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 52b35c931..19c956079 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -19,6 +19,42 @@
:version: 1.0.1
.. change::
+ :tags: bug, sqlite
+ :tickets: 3378
+
+ Fixed a regression due to :ticket:`3282`, where due to the fact that
+ we attempt to assume the availability of ALTER when creating/dropping
+ schemas, in the case of SQLite we simply said to not worry about
+ foreign keys at all, since ALTER is not available, when creating
+ and dropping tables. This meant that the sorting of tables was
+ basically skipped in the case of SQLite, and for the vast majority
+ of SQLite use cases, this is not an issue.
+
+ However, users who were doing DROPs on SQLite
+ with tables that contained data and with referential integrity
+ turned on would then experience errors, as the
+ dependency sorting *does* matter in the case of DROP with
+ enforced constraints, when those tables have data (SQLite will still
+ happily let you create foreign keys to nonexistent tables and drop
+ tables referring to existing ones with constraints enabled, as long as
+ there's no data being referenced).
+
+ In order to maintain the new feature of :ticket:`3282` while still
+ allowing a SQLite DROP operation to maintain ordering, we now
+ do the sort with full FKs taken under consideration, and if we encounter
+ an unresolvable cycle, only *then* do we forego attempting to sort
+ the tables; we instead emit a warning and go with the unsorted list.
+ If an environment needs both ordered DROPs *and* has foreign key
+ cycles, then the warning notes they will need to restore the
+ ``use_alter`` flag to their :class:`.ForeignKey` and
+ :class:`.ForeignKeyConstraint` objects so that just those objects will
+ be omitted from the dependency sort.
+
+ .. seealso::
+
+ :ref:`feature_3282` - contains an updated note about SQLite.
+
+ .. change::
:tags: bug, core
:tickets: 3372
diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst
index 49ca37b2b..392c3f66f 100644
--- a/doc/build/changelog/migration_10.rst
+++ b/doc/build/changelog/migration_10.rst
@@ -609,8 +609,8 @@ than the integer value.
.. _feature_3282:
-The ``use_alter`` flag on ``ForeignKeyConstraint`` is no longer needed
-----------------------------------------------------------------------
+The ``use_alter`` flag on ``ForeignKeyConstraint`` is (usually) 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
@@ -629,6 +629,16 @@ The :paramref:`.ForeignKeyConstraint.use_alter` and
the same effect of establishing those constraints for which ALTER is
required during a CREATE/DROP scenario.
+As of version 1.0.1, special logic takes over in the case of SQLite, which
+does not support ALTER, in the case that during a DROP, the given tables have
+an unresolvable cycle; in this case a warning is emitted, and the tables
+are dropped with **no** ordering, which is usually fine on SQLite unless
+constraints are enabled. To resolve the warning and proceed with at least
+a partial ordering on a SQLite database, particuarly one where constraints
+are enabled, re-apply "use_alter" flags to those
+:class:`.ForeignKey` and :class:`.ForeignKeyConstraint` objects which should
+be explicitly omitted from the sort.
+
.. seealso::
:ref:`use_alter` - full description of the new behavior.
diff --git a/lib/sqlalchemy/sql/ddl.py b/lib/sqlalchemy/sql/ddl.py
index bbac6456e..a0841b13c 100644
--- a/lib/sqlalchemy/sql/ddl.py
+++ b/lib/sqlalchemy/sql/ddl.py
@@ -803,32 +803,50 @@ class SchemaDropper(DDLBase):
tables = list(metadata.tables.values())
try:
+ unsorted_tables = [t for t in tables if self._can_drop_table(t)]
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
+ unsorted_tables,
+ filter_fn=lambda constraint: False
+ if not self.dialect.supports_alter
+ or constraint.name is None
else None
)
)
except exc.CircularDependencyError as err2:
- util.raise_from_cause(
- exc.CircularDependencyError(
- err2.args[0],
- err2.cycles, err2.edges,
- msg="Can't sort tables for DROP; an "
+ if not self.dialect.supports_alter:
+ util.warn(
+ "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."
+ "dependency exists between tables: %s, and backend does "
+ "not support ALTER. To restore at least a partial sort, "
+ "apply use_alter=True to ForeignKey and "
+ "ForeignKeyConstraint "
+ "objects involved in the cycle to mark these as known "
+ "cycles that will be ignored."
% (
", ".join(sorted([t.fullname for t in err2.cycles]))
)
+ )
+ collection = [(t, ()) for t in unsorted_tables]
+ else:
+ util.raise_from_cause(
+ exc.CircularDependencyError(
+ err2.args[0],
+ 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
diff --git a/lib/sqlalchemy/testing/assertsql.py b/lib/sqlalchemy/testing/assertsql.py
index e544adad2..243493607 100644
--- a/lib/sqlalchemy/testing/assertsql.py
+++ b/lib/sqlalchemy/testing/assertsql.py
@@ -188,21 +188,27 @@ class DialectSQL(CompiledSQL):
def _compile_dialect(self, execute_observed):
return execute_observed.context.dialect
+ def _compare_no_space(self, real_stmt, received_stmt):
+ stmt = re.sub(r'[\n\t]', '', real_stmt)
+ return received_stmt == stmt
+
def _received_statement(self, execute_observed):
received_stmt, received_params = super(DialectSQL, self).\
_received_statement(execute_observed)
+
+ # TODO: why do we need this part?
for real_stmt in execute_observed.statements:
- if real_stmt.statement == received_stmt:
+ if self._compare_no_space(real_stmt.statement, received_stmt):
break
else:
raise AssertionError(
"Can't locate compiled statement %r in list of "
"statements actually invoked" % received_stmt)
+
return received_stmt, execute_observed.context.compiled_parameters
def _compare_sql(self, execute_observed, received_statement):
stmt = re.sub(r'[\n\t]', '', self.statement)
-
# convert our comparison statement to have the
# paramstyle of the received
paramstyle = execute_observed.context.dialect.paramstyle
diff --git a/lib/sqlalchemy/testing/engines.py b/lib/sqlalchemy/testing/engines.py
index 3a8303546..8bd1becbf 100644
--- a/lib/sqlalchemy/testing/engines.py
+++ b/lib/sqlalchemy/testing/engines.py
@@ -98,7 +98,14 @@ def drop_all_tables(metadata, bind):
testing_reaper.close_all()
if hasattr(bind, 'close'):
bind.close()
- metadata.drop_all(bind)
+
+ if not config.db.dialect.supports_alter:
+ from . import assertions
+ with assertions.expect_warnings(
+ "Can't sort tables", assert_=False):
+ metadata.drop_all(bind)
+ else:
+ metadata.drop_all(bind)
@decorator
diff --git a/lib/sqlalchemy/testing/util.py b/lib/sqlalchemy/testing/util.py
index 6d6fa094e..1842e58a5 100644
--- a/lib/sqlalchemy/testing/util.py
+++ b/lib/sqlalchemy/testing/util.py
@@ -185,6 +185,7 @@ def provide_metadata(fn, *args, **kw):
"""Provide bound MetaData for a single test, dropping afterwards."""
from . import config
+ from . import engines
from sqlalchemy import schema
metadata = schema.MetaData(config.db)
@@ -194,7 +195,7 @@ def provide_metadata(fn, *args, **kw):
try:
return fn(*args, **kw)
finally:
- metadata.drop_all()
+ engines.drop_all_tables(metadata, config.db)
self.metadata = prev_meta
diff --git a/test/sql/test_constraints.py b/test/sql/test_constraints.py
index d024e1a27..47f81a50c 100644
--- a/test/sql/test_constraints.py
+++ b/test/sql/test_constraints.py
@@ -8,8 +8,9 @@ from sqlalchemy.testing import fixtures, AssertsExecutionResults, \
from sqlalchemy import testing
from sqlalchemy.engine import default
from sqlalchemy.testing import engines
+from sqlalchemy.testing.assertions import expect_warnings
from sqlalchemy.testing import eq_
-from sqlalchemy.testing.assertsql import AllOf, RegexSQL, CompiledSQL
+from sqlalchemy.testing.assertsql import AllOf, RegexSQL, CompiledSQL, DialectSQL
from sqlalchemy.sql import table, column
@@ -84,9 +85,11 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
metadata.drop_all, testing.db
)
else:
-
- with self.sql_execution_asserter() as asserter:
- metadata.drop_all(testing.db, checkfirst=False)
+ with expect_warnings(
+ "Can't sort tables for DROP; an unresolvable "
+ "foreign key dependency "):
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(testing.db, checkfirst=False)
asserter.assert_(
AllOf(
@@ -109,10 +112,11 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
Column('id', Integer, primary_key=True),
Column("aid", Integer),
ForeignKeyConstraint(["aid"], ["a.id"], name="bfk"))
- self._assert_cyclic_constraint(metadata, auto=True)
+ self._assert_cyclic_constraint(
+ metadata, auto=True, sqlite_warning=True)
@testing.provide_metadata
- def test_fk_column_auto_alter_constraint_create(self):
+ def test_fk_column_auto_alter_inline_constraint_create(self):
metadata = self.metadata
Table("a", metadata,
@@ -125,7 +129,24 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
ForeignKey("a.id", name="bfk")
),
)
- self._assert_cyclic_constraint(metadata, auto=True)
+ self._assert_cyclic_constraint(
+ metadata, auto=True, sqlite_warning=True)
+
+ @testing.provide_metadata
+ def test_fk_column_use_alter_inline_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", use_alter=True)
+ ),
+ )
+ self._assert_cyclic_constraint(metadata, auto=False)
@testing.provide_metadata
def test_fk_table_use_alter_constraint_create(self):
@@ -137,9 +158,10 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
ForeignKeyConstraint(["bid"], ["b.id"])
)
Table(
- "b", metadata, Column(
- 'id', Integer, primary_key=True), Column(
- "aid", Integer), ForeignKeyConstraint(
+ "b", metadata,
+ Column('id', Integer, primary_key=True),
+ Column("aid", Integer),
+ ForeignKeyConstraint(
["aid"], ["a.id"], use_alter=True, name="bfk"))
self._assert_cyclic_constraint(metadata)
@@ -157,63 +179,42 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
ForeignKey("a.id", use_alter=True, name="bfk")
),
)
- self._assert_cyclic_constraint(metadata)
+ self._assert_cyclic_constraint(metadata, auto=False)
+
+ def _assert_cyclic_constraint(
+ self, metadata, auto=False, sqlite_warning=False):
+ if testing.db.dialect.supports_alter:
+ self._assert_cyclic_constraint_supports_alter(metadata, auto=auto)
+ else:
+ self._assert_cyclic_constraint_no_alter(
+ metadata, auto=auto, sqlite_warning=sqlite_warning)
- def _assert_cyclic_constraint(self, metadata, auto=False):
+ def _assert_cyclic_constraint_supports_alter(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)'
- ')'
- )
- )
- else:
- table_assertions.append(
- CompiledSQL(
- 'CREATE TABLE a ('
- 'id INTEGER NOT NULL, '
- 'bid INTEGER, '
- 'PRIMARY KEY (id), '
- 'FOREIGN KEY(bid) REFERENCES b (id)'
- ')'
- )
+ table_assertions = [
+ CompiledSQL('CREATE TABLE b ('
+ 'id INTEGER NOT NULL, '
+ 'aid INTEGER, '
+ 'PRIMARY KEY (id)'
+ ')'
+ ),
+ CompiledSQL(
+ 'CREATE TABLE a ('
+ 'id INTEGER NOT NULL, '
+ 'bid INTEGER, '
+ 'PRIMARY KEY (id)'
+ ')'
)
+ ]
else:
- table_assertions.append(
+ table_assertions = [
CompiledSQL('CREATE TABLE b ('
'id INTEGER NOT NULL, '
'aid INTEGER, '
'PRIMARY KEY (id)'
')'
- )
- )
- table_assertions.append(
+ ),
CompiledSQL(
'CREATE TABLE a ('
'id INTEGER NOT NULL, '
@@ -222,41 +223,238 @@ class ConstraintGenTest(fixtures.TestBase, AssertsExecutionResults):
'FOREIGN KEY(bid) REFERENCES b (id)'
')'
)
- )
+ ]
assertions = [AllOf(*table_assertions)]
- if testing.db.dialect.supports_alter:
- fk_assertions = []
+ fk_assertions = []
+ fk_assertions.append(
+ CompiledSQL('ALTER TABLE b ADD CONSTRAINT bfk '
+ 'FOREIGN KEY(aid) REFERENCES a (id)')
+ )
+ if auto:
fk_assertions.append(
- CompiledSQL('ALTER TABLE b ADD CONSTRAINT bfk '
- 'FOREIGN KEY(aid) REFERENCES a (id)')
+ CompiledSQL('ALTER TABLE a ADD '
+ 'FOREIGN KEY(bid) REFERENCES b (id)')
)
- 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 = [
+ CompiledSQL('ALTER TABLE b DROP CONSTRAINT bfk'),
+ CompiledSQL("DROP TABLE a"),
+ CompiledSQL("DROP TABLE b")
+ ]
+
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False),
+ asserter.assert_(*assertions)
+
+ def _assert_cyclic_constraint_no_alter(
+ self, metadata, auto=False, sqlite_warning=False):
+ table_assertions = []
+ if auto:
+ table_assertions.append(
+ DialectSQL(
+ 'CREATE TABLE b ('
+ 'id INTEGER NOT NULL, '
+ 'aid INTEGER, '
+ 'PRIMARY KEY (id), '
+ 'CONSTRAINT bfk FOREIGN KEY(aid) REFERENCES a (id)'
+ ')'
+ )
+ )
+ table_assertions.append(
+ DialectSQL(
+ 'CREATE TABLE a ('
+ 'id INTEGER NOT NULL, '
+ 'bid INTEGER, '
+ 'PRIMARY KEY (id), '
+ 'FOREIGN KEY(bid) REFERENCES b (id)'
+ ')'
+ )
+ )
+ else:
+ table_assertions.append(
+ DialectSQL(
+ 'CREATE TABLE b ('
+ 'id INTEGER NOT NULL, '
+ 'aid INTEGER, '
+ 'PRIMARY KEY (id), '
+ 'CONSTRAINT bfk FOREIGN KEY(aid) REFERENCES a (id)'
+ ')'
+ )
+ )
+
+ table_assertions.append(
+ DialectSQL(
+ 'CREATE TABLE a ('
+ 'id INTEGER NOT NULL, '
+ 'bid INTEGER, '
+ 'PRIMARY KEY (id), '
+ 'FOREIGN KEY(bid) REFERENCES b (id)'
+ ')'
)
- assertions.append(AllOf(*fk_assertions))
+ )
+
+ assertions = [AllOf(*table_assertions)]
with self.sql_execution_asserter() as asserter:
metadata.create_all(checkfirst=False)
asserter.assert_(*assertions)
+ assertions = [AllOf(
+ CompiledSQL("DROP TABLE a"),
+ CompiledSQL("DROP TABLE b")
+ )]
+
+ if sqlite_warning:
+ with expect_warnings("Can't sort tables for DROP; "):
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False),
+ else:
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False),
+ asserter.assert_(*assertions)
+
+ @testing.force_drop_names("a", "b")
+ def test_cycle_unnamed_fks(self):
+ metadata = MetaData(testing.db)
+
+ 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")),
+ )
+
+ assertions = [
+ AllOf(
+ CompiledSQL(
+ 'CREATE TABLE b ('
+ 'id INTEGER NOT NULL, '
+ 'aid INTEGER, '
+ 'PRIMARY KEY (id)'
+ ')'
+ ),
+ CompiledSQL(
+ 'CREATE TABLE a ('
+ 'id INTEGER NOT NULL, '
+ 'bid INTEGER, '
+ 'PRIMARY KEY (id)'
+ ')'
+ )
+ ),
+ AllOf(
+ CompiledSQL('ALTER TABLE b ADD '
+ 'FOREIGN KEY(aid) REFERENCES a (id)'),
+ CompiledSQL('ALTER TABLE a ADD '
+ 'FOREIGN KEY(bid) REFERENCES b (id)')
+ ),
+ ]
+ with self.sql_execution_asserter() as asserter:
+ metadata.create_all(checkfirst=False)
+
if testing.db.dialect.supports_alter:
- assertions = [
- CompiledSQL('ALTER TABLE b DROP CONSTRAINT bfk'),
- CompiledSQL("DROP TABLE a"),
- CompiledSQL("DROP TABLE b")
- ]
+ asserter.assert_(*assertions)
+
+ 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, checkfirst=False
+ )
else:
- assertions = [AllOf(
- CompiledSQL("DROP TABLE a"),
- CompiledSQL("DROP TABLE b")
- )]
+ with expect_warnings(
+ "Can't sort tables for DROP; an unresolvable "
+ "foreign key dependency exists between tables"):
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False)
+
+ asserter.assert_(
+ AllOf(
+ CompiledSQL("DROP TABLE b"),
+ CompiledSQL("DROP TABLE a"),
+ )
+ )
+
+ @testing.force_drop_names("a", "b")
+ def test_cycle_named_fks(self):
+ metadata = MetaData(testing.db)
+ 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", use_alter=True, name='aidfk')),
+ )
+
+ assertions = [
+ AllOf(
+ CompiledSQL(
+ 'CREATE TABLE b ('
+ 'id INTEGER NOT NULL, '
+ 'aid INTEGER, '
+ 'PRIMARY KEY (id)'
+ ')'
+ ),
+ CompiledSQL(
+ 'CREATE TABLE a ('
+ 'id INTEGER NOT NULL, '
+ 'bid INTEGER, '
+ 'PRIMARY KEY (id), '
+ 'FOREIGN KEY(bid) REFERENCES b (id)'
+ ')'
+ )
+ ),
+ CompiledSQL('ALTER TABLE b ADD CONSTRAINT aidfk '
+ 'FOREIGN KEY(aid) REFERENCES a (id)'),
+ ]
with self.sql_execution_asserter() as asserter:
- metadata.drop_all(checkfirst=False),
- asserter.assert_(*assertions)
+ metadata.create_all(checkfirst=False)
+
+ if testing.db.dialect.supports_alter:
+ asserter.assert_(*assertions)
+
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False)
+
+ asserter.assert_(
+ CompiledSQL("ALTER TABLE b DROP CONSTRAINT aidfk"),
+ AllOf(
+ CompiledSQL("DROP TABLE b"),
+ CompiledSQL("DROP TABLE a"),
+ )
+ )
+ else:
+ with self.sql_execution_asserter() as asserter:
+ metadata.drop_all(checkfirst=False)
+
+ asserter.assert_(
+ AllOf(
+ CompiledSQL("DROP TABLE b"),
+ CompiledSQL("DROP TABLE a"),
+ ),
+ )
+
+
+
+
+
@testing.requires.check_constraints
@testing.provide_metadata
View Lorry Controller Status