From: Mike Bayer Date: Wed, 11 Apr 2018 13:29:00 +0000 (-0400) Subject: Correct join for FKs with schema in SQL Server X-Git-Tag: rel_1_3_0b1~210^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9d5e117f6fcc38d8773bc943c615888dc8a3a819;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..f09533e549 --- /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 840c355dc0..f55f7e74df 100644 --- 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 1275cac032..70a2f3f4f8 100644 --- 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 724f361473..57c841dcf6 100644 --- 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 0cabafff2b..395344041a 100644 --- 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(