`nodes` carries two name indexes and neither can serve
`WHERE name = ? COLLATE NOCASE`: `idx_nodes_name` is BINARY-collated, and
`idx_nodes_lower_name` is an expression index the planner only matches against
the same expression. All three whole-name lookups in the query layer were
written that way, so each one degraded to a full table scan
(`EXPLAIN QUERY PLAN` reports `SCAN nodes`).
The LIMITs on those queries do not rescue them. SQLite can only stop early once
it has produced LIMIT rows, and the two dominant cases never get there: a query
word that names no symbol at all, and a name with only a handful of definitions.
`searchNodes` runs its supplement once per query term; `findNodesByExactName`
runs two passes per symbol extracted from the question, and extraction is
generous, so a plainly-worded question issues a dozen full scans.
Written as `lower(name) = lower(?)` the same predicate seeks
`idx_nodes_lower_name`. Measured on four indexed repositories, baseline vs fix
in one process (the only difference being how the predicate is spelled):
query "how does the retry backoff work" findNodesByExactName searchNodes
gin (2.5k nodes) 1.27ms -> 0.18ms 3.1 -> 2.6ms
Alamofire (4.5k nodes) 2.39ms -> 0.22ms 4.9 -> 4.0ms
excalidraw (11k nodes) 10.54ms -> 0.17ms 10.4 -> 5.8ms
django (62k nodes) 49.91ms -> 0.17ms 27.6 -> 4.9ms
The seek is flat across all four; the scan grows with the corpus. A one-word
query into `searchNodes` on django is unchanged (~20ms) because a single term's
scan is not what dominates it there.
Lowering the parameter in SQL rather than in JavaScript is deliberate. SQLite's
`lower()` and NOCASE both fold ASCII only, while JavaScript's `.toLowerCase()`
folds Unicode; comparing a JS-lowered parameter against `lower(name)` would
silently stop matching non-ASCII identifiers that NOCASE used to match.
`getNodesByLowerName` is spelled the same way for the same reason. It already
sought the index, but as a bare `lower(name) = ?` it took a pre-lowered
parameter on trust: any input carrying an uppercase letter returned nothing at
all. This is behaviour-neutral for its one caller — `matchFuzzy` lowers in
JavaScript before calling, and `lower()` over an already-lowered string is a
no-op, verified over the ASCII and non-ASCII cases alike. It closes the trap for
the next caller; the non-ASCII gap on the `matchFuzzy` side is a resolution
change and is deliberately not bundled here.
Result sets are unchanged, including which rows the LIMITs keep: entries under
one key in the expression index are ordered by rowid, the same order a table
scan produces. Verified over 14,400 lookups (top-400 names of the four
corpora, probed as stored / upper / lower, against all three call sites) with
zero differences, and end-to-end above with identical result ids.
Tests assert the planner's verdict rather than a wall-clock number, so they are
deterministic: they intercept the SQL each call site prepares and require an
index seek, with a guard that the lookups actually ran. Reverting any call site
turns them red.
Co-authored-by: Colby McHenry <me@colbymchenry.com>
207 lines
7.7 KiB
TypeScript
207 lines
7.7 KiB
TypeScript
/**
|
|
* Exact-name lookups must seek `idx_nodes_lower_name`
|
|
*
|
|
* `nodes` carries two name indexes and neither one can serve
|
|
* `WHERE name = ? COLLATE NOCASE`:
|
|
*
|
|
* - `idx_nodes_name` is BINARY-collated, so NOCASE equality can't use it;
|
|
* - `idx_nodes_lower_name` is an expression index on `lower(name)`, and the
|
|
* planner only matches it against the same expression.
|
|
*
|
|
* So every exact-name lookup written that way degrades to a full table scan.
|
|
* The `LIMIT`s on those queries do not save them: SQLite can only stop early
|
|
* once it has produced `LIMIT` rows, and the common cases — a query term that
|
|
* is not a symbol at all, or a name with only a handful of definitions — never
|
|
* reach it and scan the whole table.
|
|
*
|
|
* These tests read the planner's own verdict rather than a wall-clock number,
|
|
* so they are deterministic and fail loudly if a lookup regresses to a scan.
|
|
* `lower(name) = lower(?)` (not a JS-side `.toLowerCase()`) is the required
|
|
* form — see the folding-parity test at the bottom for why.
|
|
*/
|
|
|
|
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
|
import * as fs from 'fs';
|
|
import * as path from 'path';
|
|
import * as os from 'os';
|
|
import { DatabaseConnection } from '../src/db';
|
|
import { QueryBuilder } from '../src/db/queries';
|
|
import { SqliteDatabase } from '../src/db/sqlite-adapter';
|
|
import { Node } from '../src/types';
|
|
|
|
function makeNode(id: string, name: string, filePath = 'src/a.ts'): Node {
|
|
return {
|
|
id,
|
|
kind: 'function',
|
|
name,
|
|
qualifiedName: name,
|
|
filePath,
|
|
language: 'typescript',
|
|
startLine: 1,
|
|
endLine: 2,
|
|
startColumn: 0,
|
|
endColumn: 0,
|
|
updatedAt: Date.now(),
|
|
};
|
|
}
|
|
|
|
/** Wraps a db so every `prepare()` is recorded, then delegates unchanged. */
|
|
function recordingDb(raw: SqliteDatabase): { db: SqliteDatabase; sqls: string[] } {
|
|
const sqls: string[] = [];
|
|
const db: SqliteDatabase = {
|
|
prepare(sql: string) {
|
|
sqls.push(sql);
|
|
return raw.prepare(sql);
|
|
},
|
|
exec: (sql: string) => raw.exec(sql),
|
|
pragma: (str: string, options?: { simple?: boolean }) => raw.pragma(str, options),
|
|
transaction: <T>(fn: (...args: any[]) => T) => raw.transaction(fn),
|
|
close: () => raw.close(),
|
|
get open() {
|
|
return raw.open;
|
|
},
|
|
};
|
|
return { db, sqls };
|
|
}
|
|
|
|
/** SQL that filters `nodes` on whole-name equality, in either spelling. */
|
|
function exactNameLookups(sqls: string[]): string[] {
|
|
return sqls.filter(
|
|
(s) =>
|
|
/\bFROM\s+nodes\b/i.test(s) &&
|
|
(/\bname\s*(COLLATE\s+NOCASE\s*)?=\s*\?(\s*COLLATE\s+NOCASE)?/i.test(s) ||
|
|
/\blower\(name\)\s*=/i.test(s))
|
|
);
|
|
}
|
|
|
|
/** The planner's access path for the `nodes` table in a statement. */
|
|
function nodesAccessPath(raw: SqliteDatabase, sql: string): string {
|
|
const args = new Array((sql.match(/\?/g) ?? []).length).fill('x');
|
|
const rows = raw.prepare(`EXPLAIN QUERY PLAN ${sql}`).all(...args) as { detail: string }[];
|
|
const detail = rows.map((r) => r.detail).find((d) => /\bnodes\b/.test(d));
|
|
return detail ?? rows.map((r) => r.detail).join(' | ');
|
|
}
|
|
|
|
describe('exact-name lookups seek idx_nodes_lower_name', () => {
|
|
let dir: string;
|
|
let conn: DatabaseConnection;
|
|
let raw: SqliteDatabase;
|
|
|
|
beforeAll(() => {
|
|
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'name-lookup-index-'));
|
|
conn = DatabaseConnection.initialize(path.join(dir, 'test.db'));
|
|
raw = conn.getDb();
|
|
const seed = new QueryBuilder(raw);
|
|
|
|
// A corpus wide enough that a scan and a seek can't accidentally agree on
|
|
// ordering, with `handleRequest` deliberately rare (2 nodes) — the shape
|
|
// the LIMITs never short-circuit on.
|
|
const nodes: Node[] = [];
|
|
for (let i = 0; i < 300; i++) {
|
|
nodes.push(makeNode(`filler-${i}`, `filler${i}Symbol`, `src/pkg${i % 7}/f${i}.ts`));
|
|
}
|
|
nodes.push(makeNode('hr-1', 'handleRequest', 'src/server/router.ts'));
|
|
nodes.push(makeNode('hr-2', 'HandleRequest', 'src/server/legacy.ts'));
|
|
for (const n of nodes) seed.insertNode(n);
|
|
});
|
|
|
|
afterAll(() => {
|
|
conn.close();
|
|
fs.rmSync(dir, { recursive: true, force: true });
|
|
});
|
|
|
|
it('searchNodes issues its exact-name supplement as an index seek', () => {
|
|
const { db, sqls } = recordingDb(raw);
|
|
const q = new QueryBuilder(db);
|
|
|
|
const results = q.searchNodes('handleRequest');
|
|
expect(results.length).toBeGreaterThan(0);
|
|
|
|
const lookups = exactNameLookups(sqls);
|
|
// Guard against a vacuous pass: the supplement must actually have run.
|
|
expect(lookups.length).toBeGreaterThan(0);
|
|
|
|
for (const sql of lookups) {
|
|
expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
|
|
}
|
|
});
|
|
|
|
it('findNodesByExactName issues both of its passes as index seeks', () => {
|
|
const { db, sqls } = recordingDb(raw);
|
|
const q = new QueryBuilder(db);
|
|
|
|
const results = q.findNodesByExactName(['handleRequest']);
|
|
expect(results.length).toBeGreaterThan(0);
|
|
|
|
const lookups = exactNameLookups(sqls);
|
|
// Two passes: the file_path probe and the row fetch.
|
|
expect(lookups.length).toBeGreaterThanOrEqual(2);
|
|
|
|
for (const sql of lookups) {
|
|
expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
|
|
}
|
|
});
|
|
|
|
it('getNodesByLowerName seeks the index and does not depend on the caller lowering', () => {
|
|
const { db, sqls } = recordingDb(raw);
|
|
const q = new QueryBuilder(db);
|
|
|
|
// Previously this took an already-lowered string on trust: anything with an
|
|
// uppercase letter in it silently returned nothing.
|
|
expect(q.getNodesByLowerName('handlerequest').map((n) => n.id).sort()).toEqual([
|
|
'hr-1',
|
|
'hr-2',
|
|
]);
|
|
expect(q.getNodesByLowerName('HandleRequest').map((n) => n.id).sort()).toEqual([
|
|
'hr-1',
|
|
'hr-2',
|
|
]);
|
|
expect(q.getNodesByLowerName('HANDLEREQUEST').map((n) => n.id).sort()).toEqual([
|
|
'hr-1',
|
|
'hr-2',
|
|
]);
|
|
|
|
const lookups = exactNameLookups(sqls);
|
|
expect(lookups.length).toBeGreaterThan(0);
|
|
for (const sql of lookups) {
|
|
expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
|
|
}
|
|
});
|
|
|
|
it('still matches case-insensitively across both call sites', () => {
|
|
const q = new QueryBuilder(raw);
|
|
|
|
const exact = q.findNodesByExactName(['HANDLEREQUEST']);
|
|
expect(exact.map((r) => r.node.id).sort()).toEqual(['hr-1', 'hr-2']);
|
|
|
|
const searched = q.searchNodes('HandleRequest');
|
|
const ids = new Set(searched.map((r) => r.node.id));
|
|
expect(ids.has('hr-1')).toBe(true);
|
|
expect(ids.has('hr-2')).toBe(true);
|
|
});
|
|
|
|
it('folds exactly what COLLATE NOCASE folded — ASCII only', () => {
|
|
// SQLite's NOCASE and its `lower()` are both ASCII-only. JavaScript's
|
|
// `.toLowerCase()` is not, so lowering the parameter in JS and comparing
|
|
// against `lower(name)` would silently stop matching non-ASCII names that
|
|
// NOCASE used to match. `lower(?)` keeps both sides on SQLite's rules.
|
|
const probe = new QueryBuilder(raw);
|
|
probe.insertNode(makeNode('uni-1', 'Ünïcode', 'src/i18n/a.ts'));
|
|
|
|
const found = probe.findNodesByExactName(['Ünïcode']);
|
|
expect(found.map((r) => r.node.id)).toContain('uni-1');
|
|
|
|
// The mixed-ASCII half still folds, as NOCASE did.
|
|
probe.insertNode(makeNode('uni-2', 'Ünïcodeloader', 'src/i18n/b.ts'));
|
|
const folded = probe.findNodesByExactName(['ÜnïcodeLOADER']);
|
|
expect(folded.map((r) => r.node.id)).toContain('uni-2');
|
|
|
|
// Same rule for the fuzzy-match lookup. Note what this does NOT claim: a
|
|
// caller that lowers in JavaScript first still hands over `ünïcode`, which
|
|
// is not what SQLite's `lower()` makes of `Ünïcode`, so the gap stays open
|
|
// on that side.
|
|
expect(probe.getNodesByLowerName('Ünïcode').map((n) => n.id)).toContain('uni-1');
|
|
expect(probe.getNodesByLowerName('ÜnïcodeLOADER').map((n) => n.id)).toContain('uni-2');
|
|
});
|
|
});
|