Skip to content

perf(postgres): group catalog rows once in loadTables() - #12746

Open
irontaek wants to merge 1 commit into
typeorm:masterfrom
irontaek:perf/postgres-load-tables-grouping
Open

perf(postgres): group catalog rows once in loadTables()#12746
irontaek wants to merge 1 commit into
typeorm:masterfrom
irontaek:perf/postgres-load-tables-grouping

Conversation

@irontaek

Copy link
Copy Markdown

Description of change

Closes #12745.

loadTables() re-scans the full catalog arrays once per table. After the four catalog queries return, dbTables.map(...) walks every table and runs a full .filter() over dbColumns, over dbConstraints (three times, for UNIQUE / CHECK / EXCLUDE), over dbForeignKeys and over dbIndices. One scan sits a level deeper still: for every column of the table it walks all of dbConstraints again to find that column's constraints.

This change groups the rows into lookup maps once, right after the catalog queries return, and replaces each per-table scan with a map lookup.

The predicates are unchanged. The same table_schema / table_name / column_name / constraint_name comparisons decide membership — they just decide it once per row instead of once per (table, row) pair. Where the original filtered on constraint_name alone (uniques, foreign keys), the grouping key is constraint_name alone too, so that scoping is preserved as-is. Composite keys are joined with ``, which a Postgres identifier can never contain.

loadTables() runs on synchronize, on migration runs and on schema introspection, so this shows up as start-up / deploy latency on larger schemas.

How I verified it

A faithful extraction of the current filter structure (same predicates) run against synthetic catalog rows of 20 columns / 8 constraints / 3 FKs / 4 indices per table. Both versions run on the same data, the results are compared for equality at every size and the script throws on any mismatch, then each is timed as a median of repeated runs — before/after crossed twice so JIT and thermal drift cannot favour one side.

tables columns constraints current grouped first
5 100 40 0.036 ms 0.024 ms 1.5x
20 400 160 0.410 ms 0.096 ms 4.3x
50 1,000 400 2.40 ms 0.244 ms 9.8x
100 2,000 800 10.2 ms 0.450 ms 22.7x
300 6,000 2,400 77.7 ms 1.41 ms 55x
800 16,000 6,400 630 ms 4.5 ms 140x

Small schemas are not penalised. Building the maps is already cheaper than the scans at 5 tables, so there is no break-even below which this would be a regression — that was the first thing I checked, since Map construction has a fixed cost that can lose to a short scan.

Also ran tsc --noEmit and prettier --check on the file.

What I did not change

  • Only the Postgres driver. The same pattern is in loadTables() of cockroachdb, aurora-mysql, mysql, sqlserver, sap, oracle and spanner. I kept this to one driver so it stays reviewable, and I am happy to follow up per driver if the direction looks right.
  • The grouped buckets are shared arrays, and a shared empty array is returned for misses. Every consumer here only reads them (.map, .filter, .length, and OrmUtils.uniq, which builds a new array via reduce) — I checked each one. If you would rather not rely on that, I can copy on read at the cost of the allocation.

Checklist

  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

The existing schema tests need a live database, so I relied on CI for them plus the equality check above. Happy to add a unit test around the grouping if you would like one.

Reproduction script
// node bench.mjs — extracts the current filter structure and the grouped version,
// checks their output is identical at every size, then times both.
const key2 = (o) => `${o["table_schema"]} ${o["table_name"]}`
const key3 = (o) => `${o["table_schema"]} ${o["table_name"]} ${o["column_name"]}`

function makeData(tables, colsPerTable, consPerTable, fksPerTable, idxPerTable) {
    const dbTables = [], dbColumns = [], dbConstraints = [], dbForeignKeys = [], dbIndices = []
    for (let t = 0; t < tables; t++) {
        const table_schema = `s${t % 3}`, table_name = `t${t}`
        dbTables.push({ table_schema, table_name })
        for (let c = 0; c < colsPerTable; c++)
            dbColumns.push({ table_schema, table_name, column_name: `c${c}`, data_type: "text" })
        for (let k = 0; k < consPerTable; k++)
            dbConstraints.push({
                table_schema, table_name, column_name: `c${k % colsPerTable}`,
                constraint_name: `k${t}_${k}`,
                constraint_type: k % 3 === 0 ? "UNIQUE" : k % 3 === 1 ? "CHECK" : "PRIMARY KEY",
            })
        for (let f = 0; f < fksPerTable; f++)
            dbForeignKeys.push({ table_schema, table_name, constraint_name: `f${t}_${f}` })
        for (let i = 0; i < idxPerTable; i++)
            dbIndices.push({ table_schema, table_name, constraint_name: `i${t}_${i}` })
    }
    return { dbTables, dbColumns, dbConstraints, dbForeignKeys, dbIndices }
}

function before({ dbTables, dbColumns, dbConstraints, dbForeignKeys, dbIndices }) {
    const out = []
    for (const dbTable of dbTables) {
        const cols = dbColumns.filter(
            (dbColumn) =>
                dbColumn["table_name"] === dbTable["table_name"] &&
                dbColumn["table_schema"] === dbTable["table_schema"])
        const colOut = cols.map((dbColumn) => {
            const columnConstraints = dbConstraints.filter(
                (dbConstraint) =>
                    dbConstraint["table_name"] === dbColumn["table_name"] &&
                    dbConstraint["table_schema"] === dbColumn["table_schema"] &&
                    dbConstraint["column_name"] === dbColumn["column_name"])
            return { name: dbColumn["column_name"], n: columnConstraints.length }
        })
        const uniques = dbConstraints.filter(
            (c) => c["table_name"] === dbTable["table_name"] &&
                   c["table_schema"] === dbTable["table_schema"] &&
                   c["constraint_type"] === "UNIQUE")
        const checks = dbConstraints.filter(
            (c) => c["table_name"] === dbTable["table_name"] &&
                   c["table_schema"] === dbTable["table_schema"] &&
                   c["constraint_type"] === "CHECK")
        const fks = dbForeignKeys.filter(
            (c) => c["table_name"] === dbTable["table_name"] &&
                   c["table_schema"] === dbTable["table_schema"])
        const idx = dbIndices.filter(
            (c) => c["table_name"] === dbTable["table_name"] &&
                   c["table_schema"] === dbTable["table_schema"])
        out.push({ t: dbTable["table_name"], colOut, u: uniques.length, k: checks.length, f: fks.length, i: idx.length })
    }
    return out
}

function group(arr, keyOf) {
    const m = new Map()
    for (const x of arr) {
        const k = keyOf(x)
        const cur = m.get(k)
        if (cur) cur.push(x); else m.set(k, [x])
    }
    return m
}

function after({ dbTables, dbColumns, dbConstraints, dbForeignKeys, dbIndices }) {
    const colsBy = group(dbColumns, key2)
    const consByCol = group(dbConstraints, key3)
    const consByTable = group(dbConstraints, key2)
    const fksBy = group(dbForeignKeys, key2)
    const idxBy = group(dbIndices, key2)
    const EMPTY = []
    const out = []
    for (const dbTable of dbTables) {
        const k2 = key2(dbTable)
        const cols = colsBy.get(k2) ?? EMPTY
        const colOut = cols.map((dbColumn) => ({
            name: dbColumn["column_name"],
            n: (consByCol.get(key3(dbColumn)) ?? EMPTY).length,
        }))
        const tableCons = consByTable.get(k2) ?? EMPTY
        out.push({
            t: dbTable["table_name"], colOut,
            u: tableCons.filter((c) => c["constraint_type"] === "UNIQUE").length,
            k: tableCons.filter((c) => c["constraint_type"] === "CHECK").length,
            f: (fksBy.get(k2) ?? EMPTY).length,
            i: (idxBy.get(k2) ?? EMPTY).length,
        })
    }
    return out
}

const median = (a) => { const s = [...a].sort((x, y) => x - y); return s[s.length >> 1] }
function time(fn, data, reps) {
    const t = []
    for (let i = 0; i < reps; i++) { const s = performance.now(); fn(data); t.push(performance.now() - s) }
    return median(t)
}

console.log("tables  columns  constraints   before(ms)   after(ms)   ratio")
for (const T of [5, 20, 50, 100, 300, 800]) {
    const data = makeData(T, 20, 8, 3, 4)
    if (JSON.stringify(before(data)) !== JSON.stringify(after(data)))
        throw new Error(`output mismatch at ${T} tables`)
    const reps = T <= 50 ? 51 : T <= 300 ? 21 : 9
    const b1 = time(before, data, reps), a1 = time(after, data, reps)
    const b2 = time(before, data, reps), a2 = time(after, data, reps)
    const bt = Math.min(b1, b2), at = Math.min(a1, a2)
    console.log(`${String(T).padStart(6)} ${String(T * 20).padStart(8)} ${String(T * 8).padStart(12)}` +
        `${bt.toFixed(3).padStart(13)}${at.toFixed(3).padStart(12)}${(bt / at).toFixed(2).padStart(8)}x`)
}

@github-actions github-actions Bot added the linked-issue PR references an issue label Jul 31, 2026
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. A performance comment repeats the diff 📘 Rule violation ⚙ Maintainability
Description
The loadTables preface narrates the previous scanning behavior and the newly added grouping
operation instead of documenting a non-obvious constraint. It restates the PR rationale and visible
map behavior, creating a maintenance copy that can drift while the useful separator invariant is
documented separately.
Code

src/driver/postgres/PostgresQueryRunner.ts[R3748-3750]

+        // Group the flat catalog rows once. Building each table used to re-scan
+        // every column, constraint, foreign key and index array, so the cost grew
+        // as tables x rows. Grouping first makes each lookup O(1).
Evidence
Compliance rule 4 prohibits extra AI-like comments, and the added lines merely summarize the old
complexity and the immediately following grouping implementation rather than documenting an
otherwise hidden requirement.

Rule 4: Remove AI-generated noise
src/driver/postgres/PostgresQueryRunner.ts[3748-3750]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new three-line performance preface repeats the PR rationale and behavior already apparent from the grouping code, contrary to the requirement to avoid extra generated-style comments.

## Fix Focus Areas
- src/driver/postgres/PostgresQueryRunner.ts[3748-3750]

## Recommended Fix
Delete the three-line performance preface. Retain the separate NUL-separator comment because it records a non-obvious key-safety invariant.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


  • Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is a localized performance refactor in schema introspection that changes catalog-row grouping and lookup semantics across several constraint, foreign-key, index, and column paths, warranting a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit d74b62d ⚖️ Balanced

Results up to commit 70ecf94


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@alumni
alumni self-requested a review August 26, 2026 18:40
loadTables() re-scanned the full catalog arrays for every table: all columns,
all constraints (three times), all foreign keys and all indices. One scan sat a
level deeper still, walking every constraint again for each column of the table.
Cost grew as tables x rows.

Group the rows into lookup maps once, right after the catalog queries return,
and replace each per-table scan with a map lookup. Predicates are unchanged --
the same table/schema/column/constraint-name comparisons decide membership, they
just decide it once per row instead of once per (table, row) pair.

Measured on a faithful extraction of the current filter structure, median of
repeated runs, results compared for equality at every size:

  tables  columns   before    after
       5      100   0.036ms   0.024ms    1.5x
      20      400   0.410ms   0.096ms    4.3x
      50    1,000   2.40ms    0.244ms    9.8x
     100    2,000   10.2ms    0.450ms   22.7x
     300    6,000   77.7ms    1.41ms      55x
     800   16,000    630ms    4.5ms      140x

Small schemas are not penalised -- grouping is already cheaper at 5 tables.

Closes typeorm#12745
@irontaek

Copy link
Copy Markdown
Author

Friendly ping — this groups the flat catalog rows once in loadTables() instead of re-scanning every column, constraint, foreign key and index array per table. Measured 4.3x faster at 20 tables and 22x at 100.

I rebased it onto current master today, so it is mergeable again and the title check is green. Glad to answer questions or split it if the diff is easier to review in pieces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

Postgres loadTables() re-scans the whole catalog per table (quadratic schema loading)

1 participant