Correct join for FKs with schema in SQL Server

Fixed 1.2 regression caused by :ticket:`4060` where the query used to
reflect SQL Server cross-schema foreign keys was limiting the criteria
incorrectly.

Additionally, added some rework of the inter-schema reflection tests
so that MySQL, MSSQL can be included, breaking out some of the
Postgresql-specific behaviors into separate requirements.

Fixes: #4234
Change-Id: I20c8e70707075f1767b79127c2c27d4b313c6515

diff --git a/doc/build/changelog/unreleased_12/4234.rst b/doc/build/changelog/unreleased_12/4234.rst
new file mode 100644 (file)
index 0000000..f09533e
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4234.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 4234
+    :versions: 1.3.0b1
+
+    Fixed 1.2 regression caused by :ticket:`4060` where the query used to
+    reflect SQL Server cross-schema foreign keys was limiting the criteria
+    incorrectly.
+
+

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index 840c355dc0d74c9671bb97f111f84e1f176e374a..f55f7e74df48d11a33afb0d8c46c6239f61c4fee 100644 (file)
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -2241,7 +2241,7 @@ class MSDialect(default.DefaultDialect):
                         RR.c.delete_rule],
                        sql.and_(C.c.table_name == tablename,
                                 C.c.table_schema == owner,
-                                R.c.table_schema == C.c.table_schema,
+                                RR.c.constraint_schema == C.c.table_schema,
                                 C.c.constraint_name == RR.c.constraint_name,
                                 R.c.constraint_name ==
                                 RR.c.unique_constraint_name,

diff --git a/lib/sqlalchemy/testing/suite/test_reflection.py b/lib/sqlalchemy/testing/suite/test_reflection.py
index 1275cac032e379167b8149c95cfb065bed5e0562..70a2f3f4f8180d9143123efd989966e7647703f2 100644 (file)
--- a/lib/sqlalchemy/testing/suite/test_reflection.py
+++ b/lib/sqlalchemy/testing/suite/test_reflection.py
@@ -112,6 +112,43 @@ class ComponentReflectionTest(fixtures.TablesTest):
               schema=schema,
               comment=r"""the test % ' " \ table comment""")
 
+        if testing.requires.cross_schema_fk_reflection.enabled:
+            if schema is None:
+                Table(
+                    'local_table', metadata,
+                    Column('id', sa.Integer, primary_key=True),
+                    Column('data', sa.String(20)),
+                    Column(
+                        'remote_id',
+                        ForeignKey(
+                            '%s.remote_table_2.id' %
+                            testing.config.test_schema)
+                    ),
+                    test_needs_fk=True,
+                    schema=config.db.dialect.default_schema_name
+                )
+            else:
+                Table(
+                    'remote_table', metadata,
+                    Column('id', sa.Integer, primary_key=True),
+                    Column(
+                        'local_id',
+                        ForeignKey(
+                            '%s.local_table.id' %
+                            config.db.dialect.default_schema_name)
+                    ),
+                    Column('data', sa.String(20)),
+                    schema=schema,
+                    test_needs_fk=True,
+                )
+                Table(
+                    'remote_table_2', metadata,
+                    Column('id', sa.Integer, primary_key=True),
+                    Column('data', sa.String(20)),
+                    schema=schema,
+                    test_needs_fk=True,
+                )
+
         if testing.requires.index_reflection.enabled:
             cls.define_index(metadata, users)
 
@@ -216,7 +253,8 @@ class ComponentReflectionTest(fixtures.TablesTest):
     def _test_get_table_names(self, schema=None, table_type='table',
                               order_by=None):
         _ignore_tables = [
-            'comment_test', 'noncol_idx_test_pk', 'noncol_idx_test_nopk'
+            'comment_test', 'noncol_idx_test_pk', 'noncol_idx_test_nopk',
+            'local_table', 'remote_table', 'remote_table_2'
         ]
         meta = self.metadata
         users, addresses, dingalings = self.tables.users, \
@@ -540,6 +578,41 @@ class ComponentReflectionTest(fixtures.TablesTest):
     def test_get_foreign_keys_with_schema(self):
         self._test_get_foreign_keys(schema=testing.config.test_schema)
 
+    @testing.requires.cross_schema_fk_reflection
+    @testing.requires.schemas
+    def test_get_inter_schema_foreign_keys(self):
+        local_table, remote_table, remote_table_2 = self.tables(
+            '%s.local_table' % testing.db.dialect.default_schema_name,
+            '%s.remote_table' % testing.config.test_schema,
+            '%s.remote_table_2' % testing.config.test_schema
+        )
+
+        insp = inspect(config.db)
+
+        local_fkeys = insp.get_foreign_keys(local_table.name)
+        eq_(len(local_fkeys), 1)
+
+        fkey1 = local_fkeys[0]
+        eq_(fkey1['referred_schema'], testing.config.test_schema)
+        eq_(fkey1['referred_table'], remote_table_2.name)
+        eq_(fkey1['referred_columns'], ['id', ])
+        eq_(fkey1['constrained_columns'], ['remote_id'])
+
+        remote_fkeys = insp.get_foreign_keys(
+            remote_table.name, schema=testing.config.test_schema)
+        eq_(len(remote_fkeys), 1)
+
+        fkey2 = remote_fkeys[0]
+
+        assert fkey2['referred_schema'] in (
+            None,
+            testing.db.dialect.default_schema_name
+        )
+        eq_(fkey2['referred_table'], local_table.name)
+        eq_(fkey2['referred_columns'], ['id', ])
+        eq_(fkey2['constrained_columns'], ['local_id'])
+
+
     @testing.requires.foreign_key_constraint_option_reflection_ondelete
     def test_get_foreign_key_options_ondelete(self):
         self._test_get_foreign_key_options(ondelete="CASCADE")

diff --git a/test/engine/test_reflection.py b/test/engine/test_reflection.py
index 724f36147318655c24c2974465ee2b80c47946c7..57c841dcf6b0da6e22b5189a6808d603ad395f91 100644 (file)
--- a/test/engine/test_reflection.py
+++ b/test/engine/test_reflection.py
@@ -1397,6 +1397,10 @@ class SchemaTest(fixtures.TestBase):
     @testing.requires.schemas
     @testing.requires.cross_schema_fk_reflection
     def test_has_schema(self):
+        if not hasattr(testing.db.dialect, "has_schema"):
+            testing.config.skip_test(
+                "dialect %s doesn't have a has_schema method" %
+                testing.db.dialect.name)
         eq_(testing.db.dialect.has_schema(testing.db,
                                           testing.config.test_schema), True)
         eq_(testing.db.dialect.has_schema(testing.db,
@@ -1404,6 +1408,7 @@ class SchemaTest(fixtures.TestBase):
 
     @testing.requires.schemas
     @testing.requires.cross_schema_fk_reflection
+    @testing.requires.implicit_default_schema
     @testing.provide_metadata
     def test_blank_schema_arg(self):
         metadata = self.metadata
@@ -1411,11 +1416,13 @@ class SchemaTest(fixtures.TestBase):
         Table('some_table', metadata,
               Column('id', Integer, primary_key=True),
               Column('sid', Integer, sa.ForeignKey('some_other_table.id')),
-              schema=testing.config.test_schema
+              schema=testing.config.test_schema,
+              test_needs_fk=True
               )
         Table('some_other_table', metadata,
               Column('id', Integer, primary_key=True),
-              schema=None
+              schema=None,
+              test_needs_fk=True
               )
         metadata.create_all()
         with testing.db.connect() as conn:
@@ -1536,6 +1543,7 @@ class SchemaTest(fixtures.TestBase):
 
     @testing.requires.schemas
     @testing.requires.cross_schema_fk_reflection
+    @testing.requires.implicit_default_schema
     @testing.provide_metadata
     def test_reflect_all_schemas_default_overlap(self):
         t1 = Table('t', self.metadata,

diff --git a/test/requirements.py b/test/requirements.py
index 0cabafff2bd8a8089ac7b467f226fd30ebb4f2ae..395344041aab5a48aaab9d17f3b7832756287d2d 100644 (file)
--- a/test/requirements.py
+++ b/test/requirements.py
@@ -383,9 +383,24 @@ class DefaultRequirements(SuiteRequirements):
         """target system must support reflection of inter-schema foreign keys
         """
         return only_on([
-                    "postgresql"
+                    "postgresql",
+                    "mysql",
+                    "mssql",
                 ])
 
+    @property
+    def implicit_default_schema(self):
+        """target system has a strong concept of 'default' schema that can
+           be referred to implicitly.
+
+           basically, Postgresql.
+
+        """
+        return only_on([
+                    "postgresql",
+                ])
+
+
     @property
     def unique_constraint_reflection(self):
         return fails_on_everything_except(