Foreign key constraints not reflected from SQL Server Database

[sqlalchemy-bot]

Migrated issue, originally created by Dave Hirschfeld

Assuming a SQL Server database with two schemas TEST1 and TEST2 the below MVCE demonstrates that the foreign key constraint isn't correctly reflected from the database:

import sqlalchemy as sa

engine = sa.create_engine(
    f"mssql://{server}/{database}?driver=ODBC+Driver+13+for+SQL+Server"
)

metadata = sa.MetaData()

table1 = sa.Table(
    'TrueFalse', metadata,
    sa.Column('id', sa.SmallInteger, primary_key=True),
    sa.Column('name', sa.String(5), nullable=False),
    schema='TEST2',
)

table2 = sa.Table(
    'TestTable', metadata,
    sa.Column('id', sa.Integer, primary_key=True),
    sa.Column('true_false', sa.SmallInteger, sa.ForeignKey(table1.c.id)),
    schema='TEST1',
)

metadata.create_all(engine)

table3 = sa.Table('TestTable', sa.MetaData(bind=engine), schema='TEST1', autoload=True)

In [10]: table2.foreign_keys
Out[10]: {ForeignKey('TEST2.TrueFalse.id')}

In [11]: table3.foreign_keys
Out[11]: set()          <---------- SHOULD BE THE SAME AS TABLE2!!!

The FK constraint is created correctly in the database.

[sqlalchemy-bot]

Dave Hirschfeld wrote:

Tested with sqlalchemy 1.2.6 against SQL Server versions 10.50.6560.0 and 14.0.1000.169

[sqlalchemy-bot]

Dave Hirschfeld wrote:

Tested against sqlalchemy 1.1.13 and the code works correctly with that version:

In [2]: table2.foreign_keys
Out[2]: {ForeignKey('TEST2.TrueFalse.id')}

In [3]: table3.foreign_keys
Out[3]: {ForeignKey('TEST2.TrueFalse.id')}

In [4]: sa.__version__
Out[4]: '1.1.13'

[sqlalchemy-bot]

Dave Hirschfeld wrote:

It appears it started failing from 1.2.0 onwards:

In [2]: table2.foreign_keys
Out[2]: {ForeignKey('TEST2.TrueFalse.id')}

In [3]: table3.foreign_keys
Out[3]: set()

In [4]: sa.__version__
Out[4]: '1.2.0'

Issue #4060 seems a likely culprit?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

you want to confirm on your end what im doing:

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index 840c355dc..f55f7e74d 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,

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

the fix is simple https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/722 but the hard part is getting the test suite to open up inter-schema reflection tests beyond just postgresql so this may take a few passes.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Correct join for FKs with schema in SQL Server

Fixed 1.2 regression caused by 🎫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

→ 9d5e117

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Correct join for FKs with schema in SQL Server

Fixed 1.2 regression caused by 🎫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
(cherry picked from commit 9d5e117)

→ 92310c1

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Dave Hirschfeld wrote:

Thanks for the quick response Mike! I can confirm that your one-line fix above appears to solve the problem for me 👍