Declare the migration-created indexes on their entities

[davidleomay]

Why

PRs #4480, #4484 and #4497 added eleven database indexes via migration, in raw SQL, without declaring them on the corresponding TypeORM entities.

That is not cosmetic. RdbmsSchemaBuilder.dropOldIndices drops any database index that has no same-named entry in the entity metadata:

// node_modules/typeorm/schema-builder/RdbmsSchemaBuilder.js
const indexMetadata = metadata.indices.find((index) => index.name === tableIndex.name);
if (!indexMetadata) return true;   // -> DROP

So a schema-generation run would have removed all eleven and silently undone those PRs. synchronize is env-gated (src/config/config.ts, SQL_SYNCHRONIZE) and off in deployed infra, but npm run migration generates from the same entity diff — it would have emitted eleven spurious DROP INDEX statements into the next migration anyone generated.

What changed

Nine entities gain class-level @Index declarations for the indexes their migrations already created:

Migration	Indexes
1785460000000-AddLedgerContentChangeScanIndexes	nine (updated, id) keyset indexes for the per-minute ledger content-change scan
1785470000000-AddTradingOrderCreatedIndex	trading_order (created)
1785510000000-AddTradingOrderRuleIdIndex	trading_order (tradingRuleId, id)

This is a no-op against the database. Those migrations deliberately used TypeORM's deterministic naming (IDX_ + first 26 hex of sha1(table + '_' + columns.sort().join('_'))), so the decorators reproduce the existing names byte-for-byte. Verified against real EntityMetadata — resolved name, ordered databaseNames and uniqueness match the SQL for all eleven, including the relation form @Index((o) => [o.tradingRule, o.id]), which TypeORM resolves to (tradingRuleId, id).

Deliberately not declared

1785520000000's log index is a covering index — INCLUDE ("totalBalanceChf", "btcPriceChf") — which TypeORM cannot express. Declaring its five key columns would be worse than leaving it alone: Postgres counts INCLUDE columns in pg_index.indkey, so TypeORM's loader reports seven columns against a five-column declaration, dropOldIndices drops on the count mismatch, and createNewIndices recreates it without the INCLUDE — silently losing the index-only scan the migration was written for. Recorded as migration-owned on the entity instead.

The regression guard

src/shared/utils/__tests__/migration-index-parity.spec.ts sweeps every migration for CREATE INDEX (applying DROP/CREATE in source order), builds real TypeORM entity metadata, and compares resolved names, column order, databaseNames and uniqueness. Reading expectations out of the SQL rather than restating them is what makes it independent of how a decorator happens to be written — it sees through relations, inherited IEntity columns and @Column({ name }) overrides.

A curated list of migrations was the first design and was wrong: it silently stops covering each new index migration, which is precisely how this drift got in. Two more index migrations landed on develop while this branch was open.

Ten indexes are exempt, each with a recorded reason, and the exemption list is self-retiring — declare one and its entry must be removed or the spec fails. Three of them are pre-existing drift on develop (virtual_iban_issuance_intent, where the entity declares the same columns but TypeORM generates a different name, so the migration's index is orphaned); fixing those belongs to the owning subdomain.

Also

payout-order.entity.spec.ts selected its index positionally (.find(i => typeof i.columns === 'function')). Class decorators apply bottom-up, so the new (updated, id) decorator registers first and that lookup would have returned the wrong index, failing expect(index.unique).toBe(true). Now selects by resolved columns.

[davidleomay]

7 full review passes, two independent lenses per pass — correctness/schema-safety and conformance/regression — each re-run from scratch against the merge base rather than the incremental delta.

I had declared the log covering index. Both lenses independently established that this was worse than not declaring it: the five-column declaration collides by name with the seven-column index Postgres reports (INCLUDE columns count in pg_index.indkey), so dropOldIndices drops it and createNewIndices recreates it without the INCLUDE. Reverted to migration-owned. The commit message's "no-op against the database" claim was false for that one index until this was caught.

The parity spec keyed entity metadata last-wins on table name. Three tables use single-table inheritance — deposit_route has four metadatas, kyc_log nine — so a leaf child overwrote the parent, which is exactly the metadata entityToSyncMetadatas filters out of schema generation. Nine indexes therefore read as undeclared when they are in fact declared on Staking, Sell, LimitRequestLog, StepLog and NameCheckLog, and I had enumerated all nine as real pre-existing drift. Now keyed on tableType !== 'entity-child', and the nine exemptions retired. The spec was also order-dependent before this: it passed only because each winning leaf happened to declare exactly the indexes not on the exemption list.

The migration sweep processed every CREATE before every DROP within a file, so a drop-then-create inside one up() left the index permanently marked dropped. That pairing is what migration:generate emits whenever an index definition changes; two live unique indexes on kyc_step and user_data were silently outside coverage. Now applied in source order.

Index columns were originally resolved through a Proxy standing in for propertiesMap. That was blind to @Column({ name }) on inherited IEntity columns — which every one of these indexes uses — and to @JoinColumn({ name }) on the relation-backed one. Replaced with real EntityMetadata; both mutations now fail.

Expectations came from a hardcoded list of migrations. Two further index migrations landed on develop while the branch was open and the list did not cover them, so the branch was rebased mid-review and the list replaced with a directory sweep.

Two assertions were tautological. The name comparison re-derived the migration's own literal from the migration's own table and columns, so it could only fail on a stray where; the schema-visibility check looked its index up by name, but synchronize: false leaves IndexMetadata.name undefined, making its primary branch unreachable. Both rewritten and each proven to fail by mutation.

The exemption rationale was wrong twice over. Six custom-named indexes were recorded as inexpressible by TypeORM — they are expressible, and a reviewer declared all six to prove it. The actual constraint is that declaring them requires passing the migration's custom name, which CONTRIBUTING forbids. Four reasons also asserted the indexes predated the naming convention; they postdate it by five to ten weeks.

payout-order.entity.spec.ts selected its index positionally, which class-decorator bottom-up registration breaks. My first fix put unique into the selector, making the uniqueness assertion tautological. Separately, the new decorator's lambda was initially exercised only by the parity spec, which left that entity below its 100% coverage-gate pin when its own spec ran in isolation.

Smaller items: a tsc error that the transpile-only test run masked, and a CREATE INDEX regex that matched a single spelling — IF NOT EXISTS, UNIQUE, CONCURRENTLY, USING btree and unquoted identifiers were all invisible to it, and the per-migration count could not detect that because it counted parsed matches rather than statements in the file.

Three latent gaps are known and left in place: a migration whose down() precedes up() would hide its CREATE INDEX statements (no current migration does this), @Entity('renamed') removes a table from the sweep via the same hatch that legitimately covers the dropped real_unit_address_confirmation table, and the metadata-count floor is loose.