- Added support for autogenerate covering the use case where :class:`.Table`

objects specified in the metadata have an explicit ``schema`` attribute
whose name matches that of the connection's default schema
(e.g. "public" for Postgresql).  Previously, it was assumed that "schema"
was ``None`` when it matched the "default" schema, now the comparison
adjusts for this. #170

diff --git a/alembic/autogenerate/api.py b/alembic/autogenerate/api.py
index 910d6eccf7a1526cbf0d5d837a37e35ddb96d443..148e352fa31a4fd65436ceffa32ddf5654bca2c2 100644 (file)
--- a/alembic/autogenerate/api.py
+++ b/alembic/autogenerate/api.py
@@ -188,13 +188,15 @@ def _produce_net_changes(connection, metadata, diffs, autogen_context,
     inspector = Inspector.from_engine(connection)
     # TODO: not hardcode alembic_version here ?
     conn_table_names = set()
+
+    default_schema = connection.dialect.default_schema_name
     if include_schemas:
         schemas = set(inspector.get_schema_names())
         # replace default schema name with None
         schemas.discard("information_schema")
         # replace the "default" schema with None
         schemas.add(None)
-        schemas.discard(connection.dialect.default_schema_name)
+        schemas.discard(default_schema)
     else:
         schemas = [None]
 

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index 49f04f12566fc1f16eae17b8f4d10b441fc53242..d932736070820052b4447236577d609ad461e3a8 100644 (file)
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -19,11 +19,37 @@ def _compare_tables(conn_table_names, metadata_table_names,
                     object_filters,
                     inspector, metadata, diffs, autogen_context):
 
+    default_schema = inspector.bind.dialect.default_schema_name
+
+    # tables coming from the connection will not have "schema"
+    # set if it matches default_schema_name; so we need a list
+    # of table names from local metadata that also have "None" if schema
+    # == default_schema_name.  Most setups will be like this anyway but
+    # some are not (see #170)
+    metadata_table_names_no_dflt_schema = OrderedSet([
+        (schema if schema != default_schema else None, tname)
+        for schema, tname in metadata_table_names
+    ])
+
+    # to adjust for the MetaData collection storing the tables either
+    # as "schemaname.tablename" or just "tablename", create a new lookup
+    # which will match the "non-default-schema" keys to the Table object.
+    tname_to_table = dict(
+                        (
+                            no_dflt_schema,
+                            metadata.tables[sa_schema._get_table_key(tname, schema)]
+                        )
+                        for no_dflt_schema, (schema, tname) in zip(
+                                    metadata_table_names_no_dflt_schema,
+                                    metadata_table_names)
+                        )
+    metadata_table_names = metadata_table_names_no_dflt_schema
+
     for s, tname in metadata_table_names.difference(conn_table_names):
         name = '%s.%s' % (s, tname) if s else tname
-        metadata_table = metadata.tables[sa_schema._get_table_key(tname, s)]
+        metadata_table = tname_to_table[(s, tname)]
         if _run_filters(metadata_table, tname, "table", False, None, object_filters):
-            diffs.append(("add_table", metadata.tables[name]))
+            diffs.append(("add_table", metadata_table))
             log.info("Detected added table %r", name)
             _compare_indexes_and_uniques(s, tname, object_filters,
                     None,
@@ -55,7 +81,7 @@ def _compare_tables(conn_table_names, metadata_table_names,
 
     for s, tname in sorted(existing_tables):
         name = '%s.%s' % (s, tname) if s else tname
-        metadata_table = metadata.tables[name]
+        metadata_table = tname_to_table[(s, tname)]
         conn_table = existing_metadata.tables[name]
 
         if _run_filters(metadata_table, tname, "table", False, conn_table, object_filters):

diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index 6140db7278ac14384de0f72e20b45d7ab88d8069..4d24d96c329b297170023aa37208902fe59b3c1c 100644 (file)
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -5,6 +5,17 @@ Changelog
 .. changelog::
     :version: 0.6.3
 
+    .. change::
+      :tags: bug
+      :tickets: 170
+
+      Added support for autogenerate covering the use case where :class:`.Table`
+      objects specified in the metadata have an explicit ``schema`` attribute
+      whose name matches that of the connection's default schema
+      (e.g. "public" for Postgresql).  Previously, it was assumed that "schema"
+      was ``None`` when it matched the "default" schema, now the comparison
+      adjusts for this.
+
     .. change::
       :tags: bug
       :pullreq: github:9

diff --git a/tests/test_autogenerate.py b/tests/test_autogenerate.py
index 49afbcb677442090636fcf2835555673f932aba1..2d41f94f96bc5fb744fdb0e3920d2f3966e10e34 100644 (file)
--- a/tests/test_autogenerate.py
+++ b/tests/test_autogenerate.py
@@ -77,6 +77,54 @@ class AutogenTest(object):
         cls.m1.drop_all(cls.bind)
         clear_staging_env()
 
+class AutogenFixtureTest(object):
+    def _fixture(self, m1, m2, include_schemas=False):
+        self.metadata, model_metadata = m1, m2
+        self.metadata.create_all(self.bind)
+
+        with self.bind.connect() as conn:
+            self.context = context = MigrationContext.configure(
+                connection=conn,
+                opts={
+                    'compare_type': True,
+                    'compare_server_default': True,
+                    'target_metadata': model_metadata,
+                    'upgrade_token': "upgrades",
+                    'downgrade_token': "downgrades",
+                    'alembic_module_prefix': 'op.',
+                    'sqlalchemy_module_prefix': 'sa.',
+                }
+            )
+
+            connection = context.bind
+            autogen_context = {
+                'imports': set(),
+                'connection': connection,
+                'dialect': connection.dialect,
+                'context': context
+                }
+            diffs = []
+            autogenerate._produce_net_changes(connection, model_metadata, diffs,
+                                              autogen_context,
+                                              object_filters=_default_object_filters,
+                                              include_schemas=include_schemas
+                                        )
+            return diffs
+
+    reports_unnamed_constraints = False
+
+    def setUp(self):
+        staging_env()
+        self.bind = self._get_bind()
+
+    def tearDown(self):
+        self.metadata.drop_all(self.bind)
+        clear_staging_env()
+
+    @classmethod
+    def _get_bind(cls):
+        return sqlite_db()
+
 
 class AutogenCrossSchemaTest(AutogenTest, TestCase):
     @classmethod
@@ -177,6 +225,62 @@ class AutogenCrossSchemaTest(AutogenTest, TestCase):
         eq_(diffs[0][1].schema, self.test_schema_name)
 
 
+class AutogenDefaultSchemaTest(AutogenFixtureTest, TestCase):
+    @classmethod
+    def _get_bind(cls):
+        cls.test_schema_name = "test_schema"
+        return db_for_dialect('postgresql')
+
+    def test_uses_explcit_schema_in_default_one(self):
+
+        default_schema = self.bind.dialect.default_schema_name
+
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table('a', m1, Column('x', String(50)))
+        Table('a', m2, Column('x', String(50)), schema=default_schema)
+
+        diffs = self._fixture(m1, m2, include_schemas=True)
+        eq_(diffs, [])
+
+    def test_uses_explcit_schema_in_default_two(self):
+
+        default_schema = self.bind.dialect.default_schema_name
+
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table('a', m1, Column('x', String(50)))
+        Table('a', m2, Column('x', String(50)), schema=default_schema)
+        Table('a', m2, Column('y', String(50)), schema="test_schema")
+
+        diffs = self._fixture(m1, m2, include_schemas=True)
+        eq_(len(diffs), 1)
+        eq_(diffs[0][0], "add_table")
+        eq_(diffs[0][1].schema, "test_schema")
+        eq_(diffs[0][1].c.keys(), ['y'])
+
+    def test_uses_explcit_schema_in_default_three(self):
+
+        default_schema = self.bind.dialect.default_schema_name
+
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table('a', m1, Column('y', String(50)), schema="test_schema")
+
+        Table('a', m2, Column('x', String(50)), schema=default_schema)
+        Table('a', m2, Column('y', String(50)), schema="test_schema")
+
+
+        diffs = self._fixture(m1, m2, include_schemas=True)
+        eq_(len(diffs), 1)
+        eq_(diffs[0][0], "add_table")
+        eq_(diffs[0][1].schema, default_schema)
+        eq_(diffs[0][1].c.keys(), ['x'])
+
+
 class ModelOne(object):
     schema = None
 
@@ -658,7 +762,8 @@ class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestCase):
     ### end Alembic commands ###""" % {"schema": self.schema})
 
 
-class AutogenerateUniqueIndexTest(TestCase):
+
+class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
 
     def test_index_flag_becomes_named_unique_constraint(self):
         m1 = MetaData()
@@ -965,54 +1070,6 @@ class AutogenerateUniqueIndexTest(TestCase):
         eq_(diffs, [])
 
 
-    def _fixture(self, m1, m2, include_schemas=False):
-        self.metadata, model_metadata = m1, m2
-        self.metadata.create_all(self.bind)
-
-        with self.bind.connect() as conn:
-            self.context = context = MigrationContext.configure(
-                connection=conn,
-                opts={
-                    'compare_type': True,
-                    'compare_server_default': True,
-                    'target_metadata': model_metadata,
-                    'upgrade_token': "upgrades",
-                    'downgrade_token': "downgrades",
-                    'alembic_module_prefix': 'op.',
-                    'sqlalchemy_module_prefix': 'sa.',
-                }
-            )
-
-            connection = context.bind
-            autogen_context = {
-                'imports': set(),
-                'connection': connection,
-                'dialect': connection.dialect,
-                'context': context
-                }
-            diffs = []
-            autogenerate._produce_net_changes(connection, model_metadata, diffs,
-                                              autogen_context,
-                                              object_filters=_default_object_filters,
-                                              include_schemas=include_schemas
-                                        )
-            return diffs
-
-    reports_unnamed_constraints = False
-
-    def setUp(self):
-        staging_env()
-        self.bind = self._get_bind()
-
-    def tearDown(self):
-        self.metadata.drop_all(self.bind)
-        clear_staging_env()
-
-    @classmethod
-    def _get_bind(cls):
-        return sqlite_db()
-
-
 
 class PGUniqueIndexTest(AutogenerateUniqueIndexTest):
     reports_unnamed_constraints = True
@@ -1080,6 +1137,7 @@ class PGUniqueIndexTest(AutogenerateUniqueIndexTest):
         eq_(diffs[0][0], "add_table")
         eq_(len(diffs), 1)
 
+
 class MySQLUniqueIndexTest(AutogenerateUniqueIndexTest):
     reports_unnamed_constraints = True