Skip to content

Commit 2ec984c

Browse files
fix(#1512): index foreign-key columns on PostgreSQL, coverage-aware
PostgreSQL never auto-indexes a foreign key's referencing (child) columns and offers no server setting to change that, so non-unique FKs were left unindexed — a performance cliff on cascades/joins and an asymmetry with MySQL/InnoDB. declare.py now emits an explicit index on the FK columns on the PostgreSQL DDL path, coverage-aware: skip when the FK columns are already a left-prefix of the child's primary key (a leading primary FK, served by the PK index), emit otherwise (secondary FKs and non-leading primary FKs). MySQL keeps InnoDB's implicit index; the unique-FK case keeps its UNIQUE INDEX. Creation only — FK-drop/alter lifecycle deferred. Tests (PostgreSQL-specific; verified failing without the fix).
1 parent afc350f commit 2ec984c

2 files changed

Lines changed: 158 additions & 2 deletions

File tree

‎src/datajoint/declare.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,27 @@ def compile_foreign_key(
301301
f"FOREIGN KEY ({fk_cols}) REFERENCES {ref_table_name} ({pk_cols}) ON UPDATE CASCADE ON DELETE RESTRICT"
302302
)
303303

304-
# declare unique index
304+
# Declare a supporting index on the foreign-key columns.
305+
#
306+
# MySQL/InnoDB creates one implicitly for every foreign key, so we emit an
307+
# explicit index only on the PostgreSQL path — Postgres never auto-indexes
308+
# the referencing (child) columns and offers no server setting to make it
309+
# (see #1512). The `unique` case keeps its UNIQUE INDEX on every backend.
310+
#
311+
# Coverage-aware: skip when the foreign-key columns are already a left-prefix
312+
# of the child's primary key, since the primary-key index then already serves
313+
# foreign-key lookups and cascades. That holds exactly for a *leading*
314+
# primary foreign key; a secondary foreign key (`primary_key is None`) or a
315+
# non-leading primary foreign key is not covered and needs its own index.
316+
fk_attrs = list(ref.primary_key)
305317
if is_unique:
306-
index_cols = ", ".join(adapter.quote_identifier(attr) for attr in ref.primary_key)
318+
index_cols = ", ".join(adapter.quote_identifier(attr) for attr in fk_attrs)
307319
index_sql.append(f"UNIQUE INDEX ({index_cols})")
320+
elif adapter.backend == "postgresql":
321+
covered_by_pk = primary_key is not None and list(primary_key[: len(fk_attrs)]) == fk_attrs
322+
if not covered_by_pk:
323+
index_cols = ", ".join(adapter.quote_identifier(attr) for attr in fk_attrs)
324+
index_sql.append(f"INDEX ({index_cols})")
308325

309326

310327
def prepare_declare(

‎tests/integration/test_fk_index.py‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
"""
2+
Tests for foreign-key supporting indexes on the PostgreSQL backend (#1512).
3+
4+
MySQL/InnoDB auto-creates an index on every foreign key's referencing columns;
5+
PostgreSQL does not. DataJoint therefore emits an explicit, coverage-aware index
6+
on the Postgres path: one is created when the FK columns are NOT already a
7+
left-prefix of the child's primary key (secondary FKs, and FKs in a non-leading
8+
position of a composite PK), and skipped when they are (a leading primary FK,
9+
already served by the PK index).
10+
11+
These tests are PostgreSQL-specific: on MySQL the equivalent index is created
12+
implicitly by InnoDB, not by DataJoint's DDL, so there is nothing of ours to
13+
assert.
14+
"""
15+
16+
import time
17+
18+
import pytest
19+
20+
import datajoint as dj
21+
22+
23+
@pytest.fixture(scope="function")
24+
def schema_by_backend(connection_by_backend, db_creds_by_backend):
25+
"""Create a fresh schema per test, parameterized across backends."""
26+
backend = db_creds_by_backend["backend"]
27+
test_id = str(int(time.time() * 1000))[-8:]
28+
schema_name = f"djtest_fkidx_{backend}_{test_id}"[:64]
29+
30+
if connection_by_backend.is_connected:
31+
try:
32+
connection_by_backend.query(
33+
f"DROP DATABASE IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)}"
34+
)
35+
except Exception:
36+
pass
37+
38+
schema = dj.Schema(schema_name, connection=connection_by_backend)
39+
yield schema
40+
41+
if connection_by_backend.is_connected:
42+
try:
43+
connection_by_backend.query(
44+
f"DROP DATABASE IF EXISTS {connection_by_backend.adapter.quote_identifier(schema_name)}"
45+
)
46+
except Exception:
47+
pass
48+
49+
50+
def _fk_support_indexes(conn, schema_name, table_name):
51+
"""Names of the DataJoint-created FK-support indexes (idx_*) on a table."""
52+
rows = conn.query(
53+
"SELECT indexname FROM pg_indexes "
54+
f"WHERE schemaname = '{schema_name}' AND tablename = '{table_name}' "
55+
"AND indexname LIKE 'idx%%'"
56+
).fetchall()
57+
return {r[0] for r in rows}
58+
59+
60+
def _indexes_covering(conn, schema_name, table_name, column):
61+
"""idx_* indexes on a table whose definition references `column`."""
62+
rows = conn.query(
63+
"SELECT indexname, indexdef FROM pg_indexes "
64+
f"WHERE schemaname = '{schema_name}' AND tablename = '{table_name}' "
65+
"AND indexname LIKE 'idx%%'"
66+
).fetchall()
67+
return {name for name, defn in rows if column in defn}
68+
69+
70+
def test_secondary_fk_gets_index(schema_by_backend, db_creds_by_backend, connection_by_backend):
71+
"""A secondary foreign key (below the ---) is not in the child PK, so its
72+
columns are unindexed by the PK; DataJoint must emit a supporting index."""
73+
if db_creds_by_backend["backend"] != "postgresql":
74+
pytest.skip("PostgreSQL-specific: MySQL/InnoDB indexes FK columns implicitly")
75+
76+
@schema_by_backend
77+
class Master(dj.Manual):
78+
definition = """
79+
master_id : int32
80+
"""
81+
82+
@schema_by_backend
83+
class Child(dj.Manual):
84+
definition = """
85+
child_id : int32
86+
---
87+
-> Master
88+
"""
89+
90+
covering = _indexes_covering(connection_by_backend, schema_by_backend.database, Child.table_name, "master_id")
91+
assert covering, "secondary FK column `master_id` must have a supporting index on PostgreSQL"
92+
93+
94+
def test_nonleading_primary_fk_gets_index(schema_by_backend, db_creds_by_backend, connection_by_backend):
95+
"""A primary FK in a non-leading position of a composite PK (PK = (b_id,
96+
a_id), a_id from the FK) is not a left-prefix of the PK index, so it needs
97+
its own index."""
98+
if db_creds_by_backend["backend"] != "postgresql":
99+
pytest.skip("PostgreSQL-specific")
100+
101+
@schema_by_backend
102+
class A(dj.Manual):
103+
definition = """
104+
a_id : int32
105+
"""
106+
107+
@schema_by_backend
108+
class B(dj.Manual):
109+
definition = """
110+
b_id : int32
111+
-> A
112+
"""
113+
114+
covering = _indexes_covering(connection_by_backend, schema_by_backend.database, B.table_name, "a_id")
115+
assert covering, "non-leading primary FK column `a_id` must have a supporting index on PostgreSQL"
116+
117+
118+
def test_leading_primary_fk_no_redundant_index(schema_by_backend, db_creds_by_backend, connection_by_backend):
119+
"""A leading primary FK (PK = (p_id, q_id), p_id from the FK) is a left-prefix
120+
of the PK index, which already serves FK lookups; no extra index is created."""
121+
if db_creds_by_backend["backend"] != "postgresql":
122+
pytest.skip("PostgreSQL-specific")
123+
124+
@schema_by_backend
125+
class P(dj.Manual):
126+
definition = """
127+
p_id : int32
128+
"""
129+
130+
@schema_by_backend
131+
class Q(dj.Manual):
132+
definition = """
133+
-> P
134+
q_id : int32
135+
"""
136+
137+
assert not _fk_support_indexes(
138+
connection_by_backend, schema_by_backend.database, Q.table_name
139+
), "a leading primary FK is covered by the PK index; no redundant FK-support index should be created"

0 commit comments

Comments
 (0)