Skip to content

Commit f06c6ba

Browse files
zzzeekzzzeek
committed
Reflect PK of referred table if referred columns not present
Fixed bug where a FOREIGN KEY that was set up to refer to the parent table by table name only without the column names would not correctly be reflected as far as setting up the "referred columns", since SQLite's PRAGMA does not report on these columns if they weren't given explicitly. For some reason this was harcoded to assume the name of the local column, which might work for some cases but is not correct. The new approach reflects the primary key of the referred table and uses the constraint columns list as the referred columns list, if the remote column(s) aren't present in the reflected pragma directly. Fixes: sqlalchemy#4810 Change-Id: I7789f83d68845ae197a782080af8ec64a7bf48cc
1 parent 2051fa2 commit f06c6ba

3 files changed

Lines changed: 135 additions & 4 deletions

File tree

‎doc/build/changelog/unreleased_13/4810.rst‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
.. change::
2+
:tags: bug, sqlite, reflection
3+
:tickets: 4810
4+
5+
Fixed bug where a FOREIGN KEY that was set up to refer to the parent table
6+
by table name only without the column names would not correctly be
7+
reflected as far as setting up the "referred columns", since SQLite's
8+
PRAGMA does not report on these columns if they weren't given explicitly.
9+
For some reason this was harcoded to assume the name of the local column,
10+
which might work for some cases but is not correct. The new approach
11+
reflects the primary key of the referred table and uses the constraint
12+
columns list as the referred columns list, if the remote column(s) aren't
13+
present in the reflected pragma directly.
14+

‎lib/sqlalchemy/dialects/sqlite/base.py‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,8 +1754,22 @@ def get_foreign_keys(self, connection, table_name, schema=None, **kw):
17541754
for row in pragma_fks:
17551755
(numerical_id, rtbl, lcol, rcol) = (row[0], row[2], row[3], row[4])
17561756

1757-
if rcol is None:
1758-
rcol = lcol
1757+
if not rcol:
1758+
# no referred column, which means it was not named in the
1759+
# original DDL. The referred columns of the foreign key
1760+
# constraint are therefore the primary key of the referred
1761+
# table.
1762+
referred_pk = self.get_pk_constraint(
1763+
connection, rtbl, schema=schema, **kw
1764+
)
1765+
# note that if table doesnt exist, we still get back a record,
1766+
# just it has no columns in it
1767+
referred_columns = referred_pk["constrained_columns"]
1768+
else:
1769+
# note we use this list only if this is the first column
1770+
# in the constraint. for subsequent columns we ignore the
1771+
# list and append "rcol" if present.
1772+
referred_columns = []
17591773

17601774
if self._broken_fk_pragma_quotes:
17611775
rtbl = re.sub(r"^[\"\[`\']|[\"\]`\']$", "", rtbl)
@@ -1768,13 +1782,15 @@ def get_foreign_keys(self, connection, table_name, schema=None, **kw):
17681782
"constrained_columns": [],
17691783
"referred_schema": schema,
17701784
"referred_table": rtbl,
1771-
"referred_columns": [],
1785+
"referred_columns": referred_columns,
17721786
"options": {},
17731787
}
17741788
fks[numerical_id] = fk
17751789

17761790
fk["constrained_columns"].append(lcol)
1777-
fk["referred_columns"].append(rcol)
1791+
1792+
if rcol:
1793+
fk["referred_columns"].append(rcol)
17781794

17791795
def fk_sig(constrained_columns, referred_table, referred_columns):
17801796
return (

‎test/dialect/test_sqlite.py‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,10 +1770,45 @@ def setup_class(cls):
17701770
")"
17711771
)
17721772

1773+
conn.execute(
1774+
"CREATE TABLE implicit_referred (pk integer primary key)"
1775+
)
1776+
# single col foreign key with no referred column given,
1777+
# must assume primary key of referred table
1778+
conn.execute(
1779+
"CREATE TABLE implicit_referrer "
1780+
"(id integer REFERENCES implicit_referred)"
1781+
)
1782+
1783+
conn.execute(
1784+
"CREATE TABLE implicit_referred_comp "
1785+
"(pk1 integer, pk2 integer, primary key (pk1, pk2))"
1786+
)
1787+
# composite foreign key with no referred columns given,
1788+
# must assume primary key of referred table
1789+
conn.execute(
1790+
"CREATE TABLE implicit_referrer_comp "
1791+
"(id1 integer, id2 integer, foreign key(id1, id2) "
1792+
"REFERENCES implicit_referred_comp)"
1793+
)
1794+
1795+
# worst case - FK that refers to nonexistent table so we cant
1796+
# get pks. requires FK pragma is turned off
1797+
conn.execute(
1798+
"CREATE TABLE implicit_referrer_comp_fake "
1799+
"(id1 integer, id2 integer, foreign key(id1, id2) "
1800+
"REFERENCES fake_table)"
1801+
)
1802+
17731803
@classmethod
17741804
def teardown_class(cls):
17751805
with testing.db.begin() as conn:
17761806
for name in [
1807+
"implicit_referrer_comp_fake",
1808+
"implicit_referrer",
1809+
"implicit_referred",
1810+
"implicit_referrer_comp",
1811+
"implicit_referred_comp",
17771812
"m",
17781813
"main.l",
17791814
"k",
@@ -1892,6 +1927,72 @@ def test_foreign_key_name_is_not_none(self):
18921927
],
18931928
)
18941929

1930+
def test_foreign_key_implicit_parent(self):
1931+
inspector = Inspector(testing.db)
1932+
fks = inspector.get_foreign_keys("implicit_referrer")
1933+
eq_(
1934+
fks,
1935+
[
1936+
{
1937+
"name": None,
1938+
"constrained_columns": ["id"],
1939+
"referred_schema": None,
1940+
"referred_table": "implicit_referred",
1941+
"referred_columns": ["pk"],
1942+
"options": {},
1943+
}
1944+
],
1945+
)
1946+
1947+
def test_foreign_key_composite_implicit_parent(self):
1948+
inspector = Inspector(testing.db)
1949+
fks = inspector.get_foreign_keys("implicit_referrer_comp")
1950+
eq_(
1951+
fks,
1952+
[
1953+
{
1954+
"name": None,
1955+
"constrained_columns": ["id1", "id2"],
1956+
"referred_schema": None,
1957+
"referred_table": "implicit_referred_comp",
1958+
"referred_columns": ["pk1", "pk2"],
1959+
"options": {},
1960+
}
1961+
],
1962+
)
1963+
1964+
def test_foreign_key_implicit_missing_parent(self):
1965+
# test when the FK refers to a non-existent table and column names
1966+
# aren't given. only sqlite allows this case to exist
1967+
inspector = Inspector(testing.db)
1968+
fks = inspector.get_foreign_keys("implicit_referrer_comp_fake")
1969+
# the referred table doesn't exist but the operation does not fail
1970+
eq_(
1971+
fks,
1972+
[
1973+
{
1974+
"name": None,
1975+
"constrained_columns": ["id1", "id2"],
1976+
"referred_schema": None,
1977+
"referred_table": "fake_table",
1978+
"referred_columns": [],
1979+
"options": {},
1980+
}
1981+
],
1982+
)
1983+
1984+
def test_foreign_key_implicit_missing_parent_reflection(self):
1985+
# full Table reflection fails however, which is not a new behavior
1986+
m = MetaData()
1987+
assert_raises_message(
1988+
exc.NoSuchTableError,
1989+
"fake_table",
1990+
Table,
1991+
"implicit_referrer_comp_fake",
1992+
m,
1993+
autoload_with=testing.db,
1994+
)
1995+
18951996
def test_unnamed_inline_foreign_key(self):
18961997
inspector = Inspector(testing.db)
18971998
fks = inspector.get_foreign_keys("e")

0 commit comments

Comments
 (0)