7392bb63 - Add the (tradingRuleId, id) index for the per-rule max-order aggregate

[TaprootFreak]

Adds a ("tradingRuleId", "id") index for the per-rule max-order aggregate on trading_order — the third sequential-scan source found after #4484 was deployed. Index only; the query is unchanged.

The finding

After #4484 shipped, the sequential-scan load on trading_order dropped from 27.1 M to 10.8 M rows per 45 s. The remainder has a third source, located by high-frequency pg_stat_activity sampling (21 of 22 hits):

SELECT MAX("tradingOrder"."id") FROM "trading_order" "tradingOrder"
INNER JOIN "trading_rule" ON "trading_rule"."id" = "tradingOrder"."tradingRuleId"
GROUP BY "tradingOrder"."tradingRuleId"

TradingRuleService.getCurrentTradingOrders, called from LogJobService — once a minute. Production EXPLAIN:

Finalize GroupAggregate → Partial HashAggregate → Parallel Seq Scan
  5,421,144 rows scanned
  Buffers: shared hit=3,734 read=85,614     (~670 MB, only 4.2 % cached)
Execution Time: 489 ms
Result: 17 rows

5.4 million rows read to produce 17 maxima — there are exactly 17 trading rules. The table holds 5,421,148 rows in 698 MB.

What this PR does, and what it no longer does

An earlier revision of this branch also rewrote the query into per-rule lookups. That half is reverted. Two reasons, both from the review:

1. Snapshot coherence. The single-statement aggregate is one READ COMMITTED snapshot, so all 17 maxima come from the same instant. Per-rule statements are N snapshots; with ~4,350 inserts a day one rule can report a maximum from before a commit and another from after — a combination the aggregate can never produce. LogJobService writes the FinanceLog from this, so that is not acceptable.

2. The coherent single-statement alternative is untestable in this repository. The tests run against pg-mem 3.0.14, which does not support correlated subqueries at all. Probed directly against pg-mem, without TypeORM in between:

OK     GROUP BY + INNER JOIN (today's form)
FAIL   correlated scalar subquery with MAX      -> column "r.id" does not exist
FAIL   LEFT JOIN LATERAL with MAX               -> column "r.id" does not exist
FAIL   correlated subquery, ORDER BY … LIMIT 1  -> column "r.id" does not exist
FAIL   CROSS JOIN LATERAL, ORDER BY … LIMIT 1   -> column "r.id" does not exist
FAIL   anti-join via NOT EXISTS                 -> column "o.tradingRuleId" does not exist
OK     DISTINCT ON

So the query stays as it is: coherent by construction, and covered by an executing test.

The index helps the unchanged query — measured, not predicted

The open question was whether the planner picks the index for GROUP BY over 17 groups, or stays on the parallel sequential scan. Answered on a PostgreSQL 17.10 rebuild with production planner settings, 5,421,152 rows at the real per-rule distribution, 730 MB:

shape	plan	blocks	time (warm)
unchanged query, no index	Parallel Seq Scan	93,486	8,181 ms
unchanged query, with index	Parallel Index Only Scan, Heap Fetches: 0	15,020	5,676 ms
DISTINCT ON, with index	Incremental Sort, spills to disk	14,865	8,853 ms
correlated subquery, with index	Index Only Scan Backward, Limit 1 per rule	52	1.5 ms

The planner does pick it: 84 % fewer block accesses, and Heap Fetches: 0 means a genuine index-only scan. The index is 116 MB.

The size is the point, more than the wall clock. In production only 4.2 % of the 698 MB table sits in the 1 GB shared_buffers, so it is re-read essentially in full every minute. A 116 MB index has a real chance of staying resident; the table demonstrably does not.

The factor left on the table

The correlated form is dramatically faster — 52 blocks against 15,020 — because it does one backward index scan per rule with an early abort. It is not taken here because pg-mem cannot execute it, so it could not be covered by a test that actually runs. DISTINCT ON would be testable but is slower than doing nothing: the mixed sort direction (tradingRuleId ascending, id descending) forces an incremental sort that spills to disk.

Recorded so it can be picked up deliberately. Replacing pg-mem for that suite is its own piece of work, not part of a performance PR.

Verification

Index name derived independently three ways (Python, shasum, and TypeORM's own DefaultNamingStrategy from this repo's node_modules) → IDX_710fd49e19d248643cb2afa70f.

format:check, lint and type-check clean; affected suites green. The test keeps its concrete per-rule id expectations, and a mutation run backs them: changing MAX to MIN turns it red, reverting turns it green. Two gaps were closed — an empty trading_rule table, and an explicit assertion that the id list handed to findBy contains no null. The tautological comparison against a re-created copy of the same aggregate was removed: with the query unchanged it compared the aggregate to itself and could no longer fail.

Caveat

All timings and plans above are externally measured — the 489 ms and the cache distribution against the production database, the plan and block counts against the rebuild. Neither is reproducible from this repository, and the migration docstring says so. The production plan should be confirmed with EXPLAIN (ANALYZE, BUFFERS) after deployment; if the planner does not pick the index, dropping it changes nothing else, since no code depends on it.

[github-actions]

❌ TypeScript: 1 errors

[TaprootFreak]

Four full review passes to reach zero findings, each with an independent conformance and logic lens. Recorded here after the merge for the trail.

Pass 1 — one merge blocker: the migration docstring opened with a table size ("~920 MB") that matched none of the measurements it cited, and carried no source attribution while every other figure had one. Removed; the measured figures further down keep their sources named.

Pass 2 — conformance clean. The logic lens, asked for a fresh look rather than a repeat, found what pass 1 had missed: the pg-mem mirror entity declared only the tradingRuleId column, no relation, while the restored query joins through a relation path. CI confirmed it — all three tests aborted with Relation with property path tradingRule in entity was not found. A second defect surfaced from the build log at the same time: the spy test cast findBy's argument onto a hand-written shape, which the type check rejected.

Pass 3 — both fixes clean, zero blockers. Both lenses independently flagged one comment that promised more than its guard delivers: it claimed "a different operator" would fail loudly, but the check is instanceof FindOperator, which Any, Not and Between all satisfy.

Pass 4 — zero blockers. The comment now states what the guard actually does.

Two things are worth stating plainly rather than leaving in the log:

The scope of this PR shrank during review. It originally paired the index with a rewrite of the query into per-rule lookups. That rewrite was withdrawn: it traded one READ COMMITTED snapshot for N, and the coherent single-statement alternative is a correlated subquery, which pg-mem cannot execute at all — so it could not be covered by a test that runs. The faster shape (52 blocks against 15,020) is recorded in the description so it can be picked up deliberately, with the test infrastructure change it needs.

The planner's choice was verified before merge rather than predicted: on a PostgreSQL 17.10 rebuild with production planner settings and the real per-rule distribution, the unchanged aggregate moves from a Parallel Seq Scan over 93,486 blocks to a Parallel Index Only Scan over 15,020 with zero heap fetches. Confirming that on production itself, with EXPLAIN (ANALYZE, BUFFERS) after deployment, is still the outstanding step — and if the planner does not pick it there, dropping the index changes nothing else, since no code depends on it.