fix(#1512): index foreign-key columns on PostgreSQL (coverage-aware) - #1526
Open
dimitri-yatsenko wants to merge 3 commits into
Open
fix(#1512): index foreign-key columns on PostgreSQL (coverage-aware)#1526dimitri-yatsenko wants to merge 3 commits into
dimitri-yatsenko wants to merge 3 commits into
Conversation
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).
The inline check only compared against the primary key (and couldn't see indexes declared after the FK line). Move the decision to a post-parse pass in prepare_declare, where the full primary key and index list are known: emit an FK support index unless its columns are a left-prefix of the primary key, of a declared index, or of a longer FK-support index already emitted (longest-first). compile_foreign_key now only records candidates. Never under-indexes; no longer creates redundant indexes. Adds a test for the declared-index coverage case.
dimitri-yatsenko
marked this pull request as ready for review
July 31, 2026 19:21
Replace the `adapter.backend == "postgresql"` string check in declare.py with a new adapter capability, `auto_indexes_foreign_keys` (MySQL True, PostgreSQL False), mirroring `supports_inline_indexes`. The backend *policy* (who indexes FK columns) now lives on the adapter; the coverage/redundancy logic stays in declare.py as backend-agnostic schema-structure reasoning. Scales to new backends without touching declare.py.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1512.
Why
PostgreSQL never auto-indexes a foreign key's referencing (child) columns and offers no server setting to change it — only MySQL/InnoDB does, implicitly. So on Postgres a non-unique FK was left with no supporting index, a performance cliff for cascade deletes, joins on the FK, and parent-existence checks on large child tables — and an asymmetry with MySQL.
What
declare.py'scompile_foreign_key()now emits an explicit index on the FK columns on the PostgreSQL DDL path only, coverage-aware:(b_id, a_id)witha_idfrom the FK), neither covered by the PK index.The coverage test is exactly
primary_key[:len(fk_attrs)] == fk_attrs. MySQL continues to rely on InnoDB's implicit index (nothing emitted); theuniqueFK case keeps itsUNIQUE INDEXon both backends. The existing Postgres DDL path already convertsINDEX (...)entries toCREATE INDEXstatements, so no new plumbing.Per the issue decision: index creation only — the index lifecycle on FK drop /
.alter()is deferred to the schema-changes design.Tests
tests/integration/test_fk_index.py(PostgreSQL-specific; MySQL cases skip since the index is InnoDB's, not ours):Verified failing without the fix (negative control: the two positive tests fail) and passing with it; full
test_declare.py+test_cascade_delete.pygreen on MySQL 8.0 and PostgreSQL 15 (52 passed, no regressions).Follow-up (not in this PR)
datajoint-docs #227 currently advises users to declare an explicit
index(...)on Postgres; once this lands, that guidance flips back to "the database indexes FK columns." I'll pair the docs update with the spec reshape.Draft pending review.