[ajitpratap0]

Summary

#565 perf(schema): remove N+1 queries and nested open cursors from the Postgres/MySQL/SQLite schema loaders

The Postgres, MySQL and SQLite schema loaders now issue four statements per Load (table list, columns, indexes, foreign keys for the whole schema, grouped by table in Go) instead of 3N+1, and LoadTable issues three; every cursor is drained and closed before the next query runs, so a MaxOpenConns=1 pool no longer deadlocks and a pooled SQLite ":memory:" database no longer returns tables with empty columns/indexes/FKs (both reproduced as failing tests against the old code). SQLite table names reach the engine as bound parameters via the pragma_* table-valued functions instead of %q-formatted PRAGMA text, and Postgres PK/UNIQUE flags are now joined on table_name AND column_name so they cannot leak between tables sharing a column name. Measured on BenchmarkSQLiteLoader_Load (count=6, benchstat): tables=100 drops from 16.7K to 9.9K allocs/op (-40.6%), 679 KiB to 250 KiB B/op (-63.3%) and 3.59 ms to 1.92 ms (-46.5%); tables=10 improves similarly. Exported API and go.mod are unchanged; the only observable output difference is that tables without foreign keys (and MySQL tables without indexes) now carry a nil slice rather than an empty one, which serializes as JSON null instead of []. Non-blocking review notes: the MySQL forTable helper falls back to any single map entry (safe only because the query is filtered to one table; LoadTable with an empty name would return an arbitrary table), and the pre-existing PG constraint_name-based pk/uq join is unchanged in scope.

Commits (1), one per issue:

• f1f5510 perf(schema): batch loader queries per schema and drop nested cursors (perf(schema): remove N+1 queries and nested open cursors from the Postgres/MySQL/SQLite schema loaders #565)

6 files changed, 1047 insertions(+), 242 deletions(-)

Test Plan

• Independent validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 120s $(go list ./... | grep -v /cbinding)), go vet, and golangci-lint on this branch.
• Each change was reviewed by an independent reviewer with access to the code and benchmarks before validation.
• Benchmarks compared with benchstat -count=6 before/after on the same machine; allocs/op and B/op are the deterministic signal, ns/op is directional on a loaded machine.

#565 benchstat

benchstat before.txt after.txt (count=6 each, byte-identical to sqlite-before/after.txt), BenchmarkSQLiteLoader_Load on Apple M4:
sec/op:    tables=10  471.8µs ±10%  -> 282.4µs ±10%  -40.16% (p=0.002 n=6)
           tables=100 3.594ms ±108% -> 1.922ms ±11%  -46.53% (p=0.002 n=6)
B/op:      tables=10  75.67Ki ±0% -> 30.09Ki ±0%  -60.23% (p=0.002 n=6)
           tables=100 679.3Ki ±0% -> 249.6Ki ±0%  -63.26% (p=0.002 n=6)
allocs/op: tables=10  1.805k ±0% -> 1.106k ±0%  -38.73% (p=0.002 n=6)
           tables=100 16.749k ±0% -> 9.947k ±0%  -40.61% (p=0.002 n=6)
tables=100 after (9.9K allocs, 250 KB, 1.9 ms) is well below the probe baseline in the plan (~17.8K allocs, ~572 KB, ~2.7 ms). No regression at tables=10. PG/MySQL: structural reduction, 4 db.Query calls per file (Load = 4 statements, LoadTable = 3), verified by grep and code read.

#565 race gate: cd /Users/ajitpratapsingh/dev/GoSQLX-worktrees/perf-schema-loader-n-plus-1 && CGO_ENABLED=1 go test -race -timeout 120s $(go list ./... | grep -v /cbinding) -> EXIT=0, 44 packages ok, 0 FAIL lines, 0 DATA RACE warnings (log: scratchpad/fix/565/validator-race.txt). Targeted -race -v -count=1 run of pkg/schema/{sqlite,postgres,mysql} + pkg/gosqlx: all 12 SQLite tests PASS (4 pre-existing + 8 new); TestPostgresLoader_Load_MaxOpenConns1, TestPostgresLoader_LoadMatchesLoadTable, TestMySQLLoader_Load_MaxOpenConns1, TestMySQLLoader_LoadMatchesLoadTable compile and SKIP with "Docker not available".

Breaking Changes

• perf(schema): remove N+1 queries and nested open cursors from the Postgres/MySQL/SQLite schema loaders #565: No exported signature changes: pkg/schema/db.Loader, and Loader/NewLoader/Load/LoadTable in pkg/schema/{postgres,mysql,sqlite} are unchanged, so no breaking change. Observable behaviour changes (all improvements or neutral): (1) Load executes 4 statements instead of 3N+1 and LoadTable 3 instead of 3+I, never with two cursors open, so a sql.DB with MaxOpenConns=1 and pooled SQLite ":memory:" now work (previously deadlock / empty results). (2) SQLite table names are passed as bound parameters to pragma_ table-valued functions instead of %q-formatted PRAGMA text, so names containing double quotes or backslashes now load correctly. (3) Tables with no indexes/FKs get nil slices where some dialects previously returned non-nil empty slices (previously inconsistent across dialects; no struct tags or in-repo serialisation; only len/range are used). Handled by documenting in the commit body; no CHANGELOG edit per ground rules.

Notes

• CHANGELOG.md and performance_baselines.json are intentionally not touched on this branch; one consolidated follow-up PR will update both for all branches of the perf audit (tracking issue perf: repo-wide optimization audit (2026-10-11) — tracking issue #566).
• Base: main.

Fixes #565
Part of #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 11, 2026 6:12pm UTC

[github-actions]

Claude finished @ajitpratap0's task in 33s —— View job

---

Review of PR #570: schema loader N+1 removal

I read the three loaders in full. I did not run the tests, because the checkout has no merge base with origin/main, so I had no diff to work from. I also did not check the test files in any depth.

Overall: this is a clean change. Each query is now a single statement per kind, every cursor is drained and closed before the next query, and the group-by-table step in Go is correct. The exported API is unchanged.

What looks good

• Single-connection safety. Each helper drains its rows and closes them via defer before returning. That fixes the MaxOpenConns=1 deadlock and the pooled :memory: empty-result bug.
• SQLite parameter binding. tableSource binds the table name with ? and builds the SQL from constants only. Names containing quotes or backslashes now work.
• Grouping logic. The "extend the last entry" merge is valid because ORDER BY keeps a constraint's rows contiguous. SQLite's FK grouping also tracks lastTable and lastID, so id collisions across tables don't merge.
• Postgres PK/UNIQUE flags. The outer join on table_name AND column_name fixes the flag leaking between tables that share a column name.

Issues and suggestions

1. SQLite NULL scans (pre-existing, but now easier to hit).
   • pkg/schema/sqlite/loader.go:187 scans ii.name into a string. Expression indexes (CREATE INDEX ... ON t(lower(x))) return NULL there, so Load fails.
   • pkg/schema/sqlite/loader.go:227 scans f."to" into a string. REFERENCES parent with no column list returns NULL there, so Load fails.
   • The old code scanned the same way, so this isn't a regression. But Load now runs one query for the whole database, so a single such index or FK makes the entire Load error out. Use sql.NullString, skip NULL index columns, and map a NULL to to an empty string or the parent's PK.
2. Postgres FK join keys (pre-existing).
   • pkg/schema/postgres/loader.go:250-258 joins kcu, ccu and rc on constraint_name only. rc has no schema or table condition.
   • Composite FKs produce a kcu×ccu cross product, so a 2-column FK yields Columns=[a,b,a,b] after the merge.
   • Constraint names that repeat across tables in the same schema can cross-contaminate. Batching now exposes every table to this at once.
   • The PR notes mention the pk/uq constraint_name join but not this one. A follow-up could join pg_constraint directly, or at least add rc.constraint_schema = tc.table_schema and match ccu/kcu on position.
3. MySQL forTable fallback (pkg/schema/mysql/loader.go:100).
   • LoadTable with tableName == "" runs the "all tables" query, because of ? = '', and then returns an arbitrary map entry. Map iteration order is random, so this is nondeterministic.
   • A cheap fix is to return an error for an empty name, or to only use the fallback when len(m) == 1.
   • Postgres and SQLite have the same "empty name means all tables" behavior in LoadTable, but they return an empty table rather than a random one.
4. MySQL FK join. rc is joined on constraint_name and constraint_schema but not table_name. MySQL allows the same FK name on different tables, so add AND rc.table_name = kcu.table_name. This is pre-existing, but the schema-wide query makes it matter more.
5. Postgres parseArrayAgg. It splits on commas, so quoted identifiers containing commas or quotes parse wrongly. Pre-existing and out of scope. Scanning with pq.Array or array_to_json would fix it.
6. nil versus empty slices. This is documented in the PR. Because the JSON output changes from [] to null, consider mentioning it in the CHANGELOG follow-up. It could break consumers that serialize Table directly.
7. Tests. The Postgres and MySQL tests skip without Docker, so CI may never run them. The SQLite tests cover the main regressions. Add cases for the SQLite expression index and the FK without a column list (item 1). Both would fail today.

Performance

The benchstat numbers look credible: about 40% fewer allocs and 63% less memory at 100 tables. The Postgres and MySQL queries are also bounded at 4 statements. The per-table pragma_* joins in SQLite are fine.

Security

No new concerns. The SQLite %q formatting is gone and every user-supplied value is bound. The remaining SQL is constant.

Overall this is a good change to merge. Item 1 is worth fixing before or soon after, and items 2–4 can be follow-ups.
| Branch: perf/schema-loader-n-plus-1

[github-actions]

Claude finished @ajitpratap0's task in 24s —— View job

---

Review of PR #570 (schema loader N+1 removal), updated for 09b7016

I read the diff of the latest commit, 09b7016, and the loaders. I couldn't run the tests in this job because the sandbox denied go test. origin/main...HEAD also has no merge base here, so I reviewed commit by commit.

Earlier feedback that 09b7016 fixes

• SQLite expression indexes. ii.name is now scanned as sql.NullString. NULL (expression) columns are skipped, but the index entry is still created, so one lower(x) index no longer fails the whole Load.
• SQLite FK without a column list. f."to" is wrapped in COALESCE(..., pragma_table_info(f."table") ... p.pk = f.seq + 1). An omitted target resolves to the parent's PK column at the same position. It stays empty only if the parent or its PK is missing, and the comment says so.
• Empty LoadTable name. All three loaders now return an error. This removes the nondeterministic MySQL forTable map-entry fallback.
• New tests. The commit adds SQLite tests for these cases (+60 lines) and empty-name tests for Postgres and MySQL.

Remaining items

None of these block the merge. All of them predate the PR, but batching exposes every table to them at once.

1. Postgres FK join keys (pkg/schema/postgres/loader.go, around lines 250-258).
   • kcu, ccu and rc join on constraint_name only.
   • Composite FKs produce a kcu×ccu cross product, so a 2-column FK can come out as Columns=[a,b,a,b].
   • Constraint names reused across tables in a schema can cross-contaminate.
   • Suggested fix: join pg_constraint directly, or at least add rc.constraint_schema = tc.table_schema and match kcu and ccu on ordinal_position and position_in_unique_constraint.
2. MySQL FK join.
   • rc doesn't constrain table_name.
   • MySQL allows the same FK name on different tables, so add AND rc.table_name = kcu.table_name.
3. Postgres parseArrayAgg. It splits on commas, so quoted identifiers that contain commas or quotes parse wrongly. Pre-existing and out of scope. array_to_json or pq.Array would fix it.
4. Postgres and MySQL tests need Docker. The MaxOpenConns1 and LoadMatchesLoadTable tests skip when Docker is missing, so CI may never run them. The SQLite tests cover the regressions. Check that some CI job has Docker, or consider adding a sqlmock check of the statement count.
5. nil versus empty slices. Tables without FKs, and MySQL tables without indexes, now serialize as JSON null instead of []. Please call this out in the consolidated CHANGELOG follow-up.

Performance and security

• The benchstat numbers are credible: 4 statements per Load and 3 per LoadTable, with about 40% fewer allocs and 63% less memory at 100 tables.
• No new security concerns. SQLite table names are bound parameters, and the SQL text is built only from constants.

Verdict

Good to merge. The earlier blocking suggestion, the SQLite NULL scans, is fixed. Items 1 and 2 are good candidates for a follow-up issue.
| Branch: perf/schema-loader-n-plus-1

[github-actions]

Claude finished @ajitpratap0's task in 14s —— View job

---

Review of PR #570: schema loader N+1 removal (HEAD 7dc6b6f)

I re-read the loaders at the current head. The latest commit is a merge of main, so nothing new has landed since the 09b7016 review. I did not run tests in this job.

Verdict: good to merge. The earlier SQLite NULL-scan and empty-name issues are fixed in 09b7016.

What looks good

• Every cursor is drained and closed (defer rows.Close()) before the next query. That fixes the MaxOpenConns=1 deadlock and the pooled :memory: empty-result bug.
• SQLite table names are bound parameters, and the SQL text is built from constants only.
• Grouping in Go is correct. Constraint rows are contiguous because of ORDER BY, and the SQLite FK grouping also tracks the table.
• The Postgres PK/UNIQUE flags now join on table_name AND column_name.
• The benchstat numbers are credible: about 40% fewer allocs and 63% less memory at 100 tables.

Follow-ups (pre-existing, non-blocking)

1. Postgres FK join (pkg/schema/postgres/loader.go, loadForeignKeys).
   • ccu and rc join on constraint_name only. rc has no schema condition, and ccu is not matched by position.
   • Composite FKs produce a kcu×ccu cross product, so a 2-column FK yields Columns=[a,b,a,b] after the merge.
   • Constraint names reused across tables can cross-contaminate.
   • Suggested fix: add rc.constraint_schema = tc.table_schema, or query pg_constraint directly.
2. MySQL FK join. rc does not constrain table_name. Add AND rc.table_name = kcu.table_name.
3. Postgres parseArrayAgg. It splits on commas, so quoted identifiers that contain commas or quotes parse wrongly.
4. Test coverage. The Postgres and MySQL MaxOpenConns1 and LoadMatchesLoadTable tests skip without Docker. Please confirm that some CI job has Docker. A sqlmock check of the statement count (4 per Load, 3 per LoadTable) would also make that gate unconditional.
5. nil versus empty slices. Tables without FKs, and MySQL tables without indexes, now serialize as JSON null instead of []. Please mention this in the consolidated CHANGELOG PR.

Security

No new concerns. User-supplied values are bound, not interpolated.
| Branch: perf/schema-loader-n-plus-1