From 5d940b714357f071413dbaabcd4ec9a5a5afb154 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Thu, 14 May 2026 16:37:16 +0100 Subject: [PATCH 1/3] Specify query parameter expansion locality task --- tasks/query-parameter-expansion-locality.md | 42 +++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tasks/query-parameter-expansion-locality.md diff --git a/tasks/query-parameter-expansion-locality.md b/tasks/query-parameter-expansion-locality.md new file mode 100644 index 00000000..4cba7509 --- /dev/null +++ b/tasks/query-parameter-expansion-locality.md @@ -0,0 +1,42 @@ +--- +status: in-progress +size: medium +--- + +# Deepen query parameter expansion locality + +## Executive summary + +Roughly 15% done. The architecture pass has selected the typegen query parameter expansion machinery as the highest-value focused improvement: it currently sits inside the broad generated query boundary module, alongside wrapper rendering, catalog writing, schema loading, and query document discovery. The missing pieces are the extraction, focused tests, verification, PR update, and alternate compare branches for the existing open PRs. + +## Assumptions + +- This run intentionally skips the normal bedtime implementation tasks and starts at the improve-codebase-architecture step. +- The normal skill pause after presenting candidate deepening opportunities is skipped because the user explicitly asked to test autonomous candidate selection. +- The branch should be based on `origin/main`; the main checkout has unrelated website changes that this task must not touch. +- Open PRs to account for after this branch lands: #111, #108, and #101. + +## Candidate selection + +The selected deepening opportunity is the query parameter expansion module inside `packages/sqlfu/src/typegen/index.ts`. + +**Problem:** `typegen/index.ts` owns too many concepts at once: generated query boundary rendering, validator emission, schema authority materialization, query document discovery, query annotation parsing, named-parameter scanning, and runtime SQL expansion. Parameter expansion is a real module with non-trivial invariants, but its interface is currently implicit in a 3k-line file. + +**Solution:** Extract query parameter expansion into a dedicated internal module with a small interface. The generated query boundary should ask that module for named parameter references, analysis SQL, runtime expansion metadata, and static/dynamic SQL rewrites without knowing how comments, strings, object parameters, and row-list inference are scanned. + +**Benefits:** This creates locality for SQL parameter parsing bugs and gives tests a direct interface instead of exercising everything through full fixture generation. It should also make future generated query boundary work easier because wrapper renderers can depend on a smaller expansion interface. + +## Checklist + +- [ ] Commit this task file by itself before implementation. +- [ ] Extract named parameter scanning and parameter expansion inference from `packages/sqlfu/src/typegen/index.ts`. +- [ ] Keep the generated query boundary interface unchanged for users. +- [ ] Add focused tests for comment/string-safe named parameter scanning and expansion inference. +- [ ] Run the relevant `sqlfu` tests and typecheck. +- [ ] Push the architecture branch and open/update the PR. +- [ ] Create replacement compare branches for open PRs #111, #108, and #101 based on this architecture branch. +- [ ] Add a table to the architecture PR body mapping each old PR to the new compare branch. + +## Implementation log + +- 2026-05-14: Created the task from `origin/main` in `../worktrees/sqlfu/improve-codebase-architecture-2026-05-14` after confirming the main checkout has unrelated website work. From db668edeb32c6ec0a9004fa9c87534e5f5ae7199 Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Thu, 14 May 2026 16:43:57 +0100 Subject: [PATCH 2/3] Extract query parameter expansion module --- packages/sqlfu/src/typegen/index.ts | 508 ++---------------- .../sqlfu/src/typegen/query-parameters.ts | 469 ++++++++++++++++ .../test/generate/query-parameters.test.ts | 126 +++++ tasks/query-parameter-expansion-locality.md | 14 +- 4 files changed, 640 insertions(+), 477 deletions(-) create mode 100644 packages/sqlfu/src/typegen/query-parameters.ts create mode 100644 packages/sqlfu/test/generate/query-parameters.test.ts diff --git a/packages/sqlfu/src/typegen/index.ts b/packages/sqlfu/src/typegen/index.ts index 7bcdae06..86e4f5a3 100644 --- a/packages/sqlfu/src/typegen/index.ts +++ b/packages/sqlfu/src/typegen/index.ts @@ -2,6 +2,17 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import {analyzeVendoredTypesqlQueries} from './analyze-vendored-typesql.js'; +import { + addParameterExpansion, + expandedFieldName, + findNamedParameterReferences, + findSqlIgnoredRanges, + parseInlineParameterExpansions, + prepareSqlForAnalysis, + replaceNamedParameters, + stripSqlComments, + type ParameterExpansion, +} from './query-parameters.js'; import { assertSqliteMaterialized, registerSqliteTypegenImpls, @@ -403,25 +414,6 @@ type QuerySource = { parameterExpansions: ParameterExpansion[]; }; -type ParameterExpansion = - | { - kind: 'scalar-array'; - name: string; - } - | { - kind: 'object-fields'; - name: string; - fields: string[]; - driverFields: string[]; - } - | { - kind: 'object-array'; - name: string; - fields: string[]; - sqlShape: 'values' | 'row-list'; - acceptsSingleOrArray: boolean; - }; - export async function materializeTypegenDatabase(input: { projectRoot: string; sourceSql: string; @@ -506,7 +498,9 @@ async function replayMigrationHistoryAsSchemaSql(config: SqlfuProjectConfig, hos ); } await using live = await openLiveDb(config.db, 'migration_history'); - const history = await Promise.resolve(readMigrationHistory(live.client, {preset: config.migrations.preset, dialect: config.dialect})); + const history = await Promise.resolve( + readMigrationHistory(live.client, {preset: config.migrations.preset, dialect: config.dialect}), + ); const migrations = await readMigrationFiles(host, config); const byName = new Map(migrations.map((migration) => [migrationName(migration), migration])); @@ -671,10 +665,6 @@ function splitQueryDocument(queryFile: QueryFile): QuerySource[] { }); } -function prepareSqlForAnalysis(sql: string, parameterExpansions: ParameterExpansion[]): string { - return stripSqlComments(applyParameterExpansionsForAnalysis(sql, parameterExpansions)); -} - type QueryAnnotation = { commentStart: number; commentEnd: number; @@ -721,18 +711,6 @@ function hasExecutableSql(sql: string): boolean { return stripSqlComments(sql).trim().length > 0; } -function stripSqlComments(sql: string): string { - const chars = sql.split(''); - for (const comment of findSqlIgnoredRanges(sql).filter((range) => range.kind !== 'string')) { - for (let index = comment.start; index < comment.end; index += 1) { - if (chars[index] !== '\n' && chars[index] !== '\r') { - chars[index] = ' '; - } - } - } - return chars.join(''); -} - function assertUniqueQueryFunctionNames(querySources: QuerySource[]): void { const seen = new Map(); for (const querySource of querySources) { @@ -747,438 +725,6 @@ function assertUniqueQueryFunctionNames(querySources: QuerySource[]): void { } } -function applyParameterExpansionsForAnalysis(sql: string, expansions: ParameterExpansion[]): string { - if (expansions.length === 0) return sql; - sql = rewriteRowListExpansionsForAnalysis(sql, expansions); - const expansionMap = new Map(expansions.map((expansion) => [expansion.name, expansion])); - return replaceNamedParameters(sql, (reference) => { - if (reference.path.length > 0) { - return `:${expandedFieldName(reference.name, reference.path[0]!)}`; - } - - const expansion = expansionMap.get(reference.name); - if (!expansion) return reference.raw; - - if (expansion.kind === 'scalar-array') { - const placeholder = `:${reference.name}`; - return reference.wrappedInParens ? placeholder : `(${placeholder})`; - } - - if (expansion.kind === 'object-fields') { - return reference.raw; - } - - const replacement = expansion.fields.map((field) => `:${expandedFieldName(expansion.name, field)}`).join(', '); - if (reference.wrappedInParens) return replacement; - return `(${replacement})`; - }); -} - -function rewriteRowListExpansionsForAnalysis(sql: string, expansions: ParameterExpansion[]): string { - let output = sql; - for (const expansion of expansions) { - if (expansion.kind !== 'object-array' || expansion.sqlShape !== 'row-list') continue; - const pattern = new RegExp(`\\(([^()]+)\\)\\s+(?:not\\s+)?in\\s*\\(\\s*:${expansion.name}\\s*\\)`, 'gi'); - output = replaceSqlPatternOutsideCommentsAndStrings(output, pattern, (match, [rawFields = '']) => { - const fields = parseSimpleSqlFieldList(rawFields, 'inferred row IN parameter'); - if (fields.join('\0') !== expansion.fields.join('\0')) return match; - const predicates = rawFields - .split(',') - .map((field) => field.trim()) - .map((field, index) => `${field} = :${expandedFieldName(expansion.name, fields[index]!)}`) - .join(' and '); - return `(${predicates})`; - }); - } - return output; -} - -type NamedParameterReference = { - raw: string; - name: string; - path: string[]; - start: number; - end: number; - wrappedInParens: boolean; -}; - -function replaceNamedParameters(sql: string, replace: (reference: NamedParameterReference) => string): string { - let output = ''; - let cursor = 0; - for (const reference of findNamedParameterReferences(sql)) { - output += sql.slice(cursor, reference.start); - output += replace(reference); - cursor = reference.end; - } - return output + sql.slice(cursor); -} - -function findNamedParameterReferences(sql: string): NamedParameterReference[] { - const references: NamedParameterReference[] = []; - let quote: "'" | '"' | '`' | null = null; - let lineComment = false; - let blockComment = false; - - for (let index = 0; index < sql.length; index += 1) { - const char = sql[index]!; - const next = sql[index + 1]; - - if (lineComment) { - if (char === '\n' || char === '\r') lineComment = false; - continue; - } - - if (blockComment) { - if (char === '*' && next === '/') { - blockComment = false; - index += 1; - } - continue; - } - - if (quote) { - if (char === quote) { - if (sql[index + 1] === quote) { - index += 1; - continue; - } - quote = null; - } - continue; - } - - if (char === '-' && next === '-') { - lineComment = true; - index += 1; - continue; - } - - if (char === '/' && next === '*') { - blockComment = true; - index += 1; - continue; - } - - if (char === "'" || char === '"' || char === '`') { - quote = char; - continue; - } - - if (char !== ':') continue; - if (next === ':') { - index += 1; - continue; - } - - const match = sql.slice(index).match(/^:([A-Za-z_$][A-Za-z0-9_$]*)/); - if (!match) continue; - - const start = index; - let cursor = index + match[0]!.length; - const referencePath: string[] = []; - while (sql[cursor] === '.') { - const fieldMatch = sql.slice(cursor).match(/^\.([A-Za-z_$][A-Za-z0-9_$]*)/); - if (!fieldMatch) break; - referencePath.push(fieldMatch[1]!); - cursor += fieldMatch[0]!.length; - } - - cursor = assertNoParameterModifier(sql, cursor); - - const raw = sql.slice(start, cursor); - references.push({ - raw, - name: match[1]!, - path: referencePath, - start, - end: cursor, - wrappedInParens: isWrappedInParens(sql, start, cursor), - }); - index = cursor - 1; - } - - return references; -} - -function assertNoParameterModifier(sql: string, index: number): number { - if (sql[index] !== ':' || sql[index + 1] === ':') return index; - - const match = sql.slice(index).match(/^:([A-Za-z_$][A-Za-z0-9_$]*)/); - throw new Error(`Unsupported parameter modifier: ${match ? match[0] : sql.slice(index, index + 1)}`); -} - -function parseInlineParameterExpansions(sql: string): ParameterExpansion[] { - const expansions = new Map(); - const references = findNamedParameterReferences(sql); - - for (const expansion of inferInsertValuesParameterExpansions(sql)) { - addParameterExpansion(expansions, expansion); - } - for (const expansion of inferRowInParameterExpansions(sql)) { - addParameterExpansion(expansions, expansion); - } - - for (const reference of references) { - if (reference.path.length > 1) { - throw new Error(`Nested parameter paths are not supported yet: ${reference.raw}`); - } - - if (reference.path.length === 1) { - const fieldName = reference.path[0]!; - addParameterExpansion(expansions, { - kind: 'object-fields', - name: reference.name, - fields: [fieldName], - driverFields: [fieldName], - }); - } - } - - for (const reference of references) { - if (reference.path.length > 0) continue; - const expansion = expansions.get(reference.name); - if (expansion?.kind === 'object-fields') { - throw new Error( - `Parameter ${JSON.stringify(reference.name)} cannot be used both as ${reference.raw} and ${expansion.kind}`, - ); - } - } - - return Array.from(expansions.values()); -} - -function inferInsertValuesParameterExpansions(sql: string): ParameterExpansion[] { - const expansions: ParameterExpansion[] = []; - const searchableSql = maskSqlCommentsAndStrings(sql); - const identifier = `[A-Za-z_$][A-Za-z0-9_$]*`; - const tableName = `${identifier}(?:\\s*\\.\\s*${identifier})?`; - const pattern = new RegExp( - `\\binsert\\s+(?:or\\s+${identifier}\\s+)?into\\s+${tableName}\\s*\\(([^)]*)\\)\\s+values\\s+:(${identifier})\\b`, - 'gi', - ); - - for (const match of searchableSql.matchAll(pattern)) { - expansions.push({ - kind: 'object-array', - name: match[2]!, - fields: parseSimpleSqlFieldList(match[1]!, 'inferred INSERT values parameter'), - sqlShape: 'values', - acceptsSingleOrArray: true, - }); - } - return expansions; -} - -function inferRowInParameterExpansions(sql: string): ParameterExpansion[] { - const expansions: ParameterExpansion[] = []; - const searchableSql = maskSqlCommentsAndStrings(sql); - const pattern = /\(([^()]+)\)\s+(?:not\s+)?in\s*\(\s*:([A-Za-z_$][A-Za-z0-9_$]*)\s*\)/gi; - - for (const match of searchableSql.matchAll(pattern)) { - const fields = parseSimpleSqlFieldList(match[1]!, 'inferred row IN parameter'); - if (fields.length < 2) continue; - expansions.push({ - kind: 'object-array', - name: match[2]!, - fields, - sqlShape: 'row-list', - acceptsSingleOrArray: false, - }); - } - return expansions; -} - -function parseSimpleSqlFieldList(rawFields: string, syntaxName: string): string[] { - const fields = rawFields - .split(',') - .map((field) => field.trim()) - .filter(Boolean); - if (fields.length === 0) { - throw new Error(`${syntaxName} needs at least one field`); - } - - const names = fields.map((field) => { - const match = field.match(/^(?:(?:[A-Za-z_$][A-Za-z0-9_$]*)\.)?([A-Za-z_$][A-Za-z0-9_$]*)$/); - if (!match) { - throw new Error(`${syntaxName} only supports simple column names: ${JSON.stringify(field)}`); - } - return match[1]!; - }); - - const duplicate = names.find((field, index) => names.indexOf(field) !== index); - if (duplicate) { - throw new Error(`${syntaxName} cannot infer duplicate field ${JSON.stringify(duplicate)}`); - } - return names; -} - -function addParameterExpansion(expansions: Map, expansion: ParameterExpansion): void { - const existing = expansions.get(expansion.name); - if (!existing) { - expansions.set(expansion.name, expansion); - return; - } - - if (existing.kind !== expansion.kind) { - throw new Error( - `Parameter ${JSON.stringify(expansion.name)} cannot use both ${existing.kind} and ${expansion.kind}`, - ); - } - - if (existing.kind === 'object-fields' && expansion.kind === 'object-fields') { - for (const fieldName of expansion.fields) { - if (!existing.fields.includes(fieldName)) { - existing.fields.push(fieldName); - } - } - existing.driverFields.push(...expansion.driverFields); - return; - } - - if (existing.kind === 'object-array' && expansion.kind === 'object-array') { - if ( - existing.fields.join('\0') !== expansion.fields.join('\0') || - existing.sqlShape !== expansion.sqlShape || - existing.acceptsSingleOrArray !== expansion.acceptsSingleOrArray - ) { - throw new Error(`Parameter ${JSON.stringify(expansion.name)} cannot use multiple inferred field sets`); - } - return; - } -} - -function isWrappedInParens(sql: string, start: number, end: number): boolean { - return previousNonWhitespace(sql, start) === '(' && nextNonWhitespace(sql, end) === ')'; -} - -function previousNonWhitespace(sql: string, index: number): string | undefined { - for (let cursor = index - 1; cursor >= 0; cursor -= 1) { - const char = sql[cursor]!; - if (!/\s/.test(char)) return char; - } - return undefined; -} - -function nextNonWhitespace(sql: string, index: number): string | undefined { - for (let cursor = index; cursor < sql.length; cursor += 1) { - const char = sql[cursor]!; - if (!/\s/.test(char)) return char; - } - return undefined; -} - -function expandedFieldName(parameterName: string, fieldName: string): string { - return `${parameterName}__${fieldName}`; -} - -type SqlIgnoredRange = { - kind: 'line-comment' | 'block-comment' | 'string'; - start: number; - end: number; -}; - -function findSqlIgnoredRanges(sql: string): SqlIgnoredRange[] { - const ranges: SqlIgnoredRange[] = []; - let quote: "'" | '"' | '`' | null = null; - let quoteStart = 0; - let lineCommentStart: number | null = null; - let blockCommentStart: number | null = null; - - for (let index = 0; index < sql.length; index += 1) { - const char = sql[index]!; - const next = sql[index + 1]; - - if (lineCommentStart !== null) { - if (char === '\n' || char === '\r') { - ranges.push({kind: 'line-comment', start: lineCommentStart, end: index}); - lineCommentStart = null; - } - continue; - } - - if (blockCommentStart !== null) { - if (char === '*' && next === '/') { - ranges.push({kind: 'block-comment', start: blockCommentStart, end: index + 2}); - blockCommentStart = null; - index += 1; - } - continue; - } - - if (quote) { - if (char === quote) { - if (next === quote) { - index += 1; - continue; - } - ranges.push({kind: 'string', start: quoteStart, end: index + 1}); - quote = null; - } - continue; - } - - if (char === '-' && next === '-') { - lineCommentStart = index; - index += 1; - continue; - } - - if (char === '/' && next === '*') { - blockCommentStart = index; - index += 1; - continue; - } - - if (char === "'" || char === '"' || char === '`') { - quote = char; - quoteStart = index; - } - } - - if (lineCommentStart !== null) { - ranges.push({kind: 'line-comment', start: lineCommentStart, end: sql.length}); - } - if (blockCommentStart !== null) { - ranges.push({kind: 'block-comment', start: blockCommentStart, end: sql.length}); - } - if (quote) { - ranges.push({kind: 'string', start: quoteStart, end: sql.length}); - } - - return ranges; -} - -function maskSqlCommentsAndStrings(sql: string): string { - const chars = sql.split(''); - for (const range of findSqlIgnoredRanges(sql)) { - for (let index = range.start; index < range.end; index += 1) { - chars[index] = ' '; - } - } - return chars.join(''); -} - -function replaceSqlPatternOutsideCommentsAndStrings( - sql: string, - pattern: RegExp, - replace: (match: string, groups: string[]) => string, -): string { - const searchableSql = maskSqlCommentsAndStrings(sql); - let output = ''; - let cursor = 0; - for (const match of searchableSql.matchAll(pattern)) { - const start = match.index!; - const end = start + match[0]!.length; - output += sql.slice(cursor, start); - output += replace( - sql.slice(start, end), - match.slice(1).map((group) => group || ''), - ); - cursor = end; - } - return output + sql.slice(cursor); -} - async function writeGeneratedQueriesFile( config: SqlfuProjectConfig, generatedDir: string, @@ -3542,11 +3088,31 @@ function extractSingleNamedParameterName(expression: string): string | undefined } function parseSimpleSqlFieldListForRefinement(rawFields: string): string[] | undefined { - try { - return parseSimpleSqlFieldList(rawFields, 'type refinement'); - } catch { + const names: string[] = []; + for (const rawField of rawFields.split(',')) { + const field = rawField.trim(); + if (!field) continue; + const match = field.match(/^(?:(?:[A-Za-z_$][A-Za-z0-9_$]*)\.)?([A-Za-z_$][A-Za-z0-9_$]*)$/); + if (!match) { + return undefined; + } + names.push(match[1]!); + } + const duplicate = names.find((field, index) => names.indexOf(field) !== index); + if (names.length === 0 || duplicate) { return undefined; } + return names; +} + +function maskSqlCommentsAndStrings(sql: string): string { + const chars = sql.split(''); + for (const range of findSqlIgnoredRanges(sql)) { + for (let index = range.start; index < range.end; index += 1) { + chars[index] = ' '; + } + } + return chars.join(''); } function inferSelectColumns( diff --git a/packages/sqlfu/src/typegen/query-parameters.ts b/packages/sqlfu/src/typegen/query-parameters.ts new file mode 100644 index 00000000..6d9090d1 --- /dev/null +++ b/packages/sqlfu/src/typegen/query-parameters.ts @@ -0,0 +1,469 @@ +export type ParameterExpansion = + | { + kind: 'scalar-array'; + name: string; + } + | { + kind: 'object-fields'; + name: string; + fields: string[]; + driverFields: string[]; + } + | { + kind: 'object-array'; + name: string; + fields: string[]; + sqlShape: 'values' | 'row-list'; + acceptsSingleOrArray: boolean; + }; + +export type NamedParameterReference = { + raw: string; + name: string; + path: string[]; + start: number; + end: number; + wrappedInParens: boolean; +}; + +export type SqlIgnoredRange = { + kind: 'line-comment' | 'block-comment' | 'string'; + start: number; + end: number; +}; + +export function prepareSqlForAnalysis(sql: string, parameterExpansions: ParameterExpansion[]): string { + return stripSqlComments(applyParameterExpansionsForAnalysis(sql, parameterExpansions)); +} + +export function applyParameterExpansionsForAnalysis(sql: string, expansions: ParameterExpansion[]): string { + if (expansions.length === 0) return sql; + sql = rewriteRowListExpansionsForAnalysis(sql, expansions); + const expansionMap = new Map(expansions.map((expansion) => [expansion.name, expansion])); + return replaceNamedParameters(sql, (reference) => { + if (reference.path.length > 0) { + return `:${expandedFieldName(reference.name, reference.path[0]!)}`; + } + + const expansion = expansionMap.get(reference.name); + if (!expansion) return reference.raw; + + if (expansion.kind === 'scalar-array') { + const placeholder = `:${reference.name}`; + return reference.wrappedInParens ? placeholder : `(${placeholder})`; + } + + if (expansion.kind === 'object-fields') { + return reference.raw; + } + + const replacement = expansion.fields.map((field) => `:${expandedFieldName(expansion.name, field)}`).join(', '); + if (reference.wrappedInParens) return replacement; + return `(${replacement})`; + }); +} + +export function replaceNamedParameters(sql: string, replace: (reference: NamedParameterReference) => string): string { + let output = ''; + let cursor = 0; + for (const reference of findNamedParameterReferences(sql)) { + output += sql.slice(cursor, reference.start); + output += replace(reference); + cursor = reference.end; + } + return output + sql.slice(cursor); +} + +export function findNamedParameterReferences(sql: string): NamedParameterReference[] { + const references: NamedParameterReference[] = []; + let quote: "'" | '"' | '`' | null = null; + let lineComment = false; + let blockComment = false; + + for (let index = 0; index < sql.length; index += 1) { + const char = sql[index]!; + const next = sql[index + 1]; + + if (lineComment) { + if (char === '\n' || char === '\r') lineComment = false; + continue; + } + + if (blockComment) { + if (char === '*' && next === '/') { + blockComment = false; + index += 1; + } + continue; + } + + if (quote) { + if (char === quote) { + if (sql[index + 1] === quote) { + index += 1; + continue; + } + quote = null; + } + continue; + } + + if (char === '-' && next === '-') { + lineComment = true; + index += 1; + continue; + } + + if (char === '/' && next === '*') { + blockComment = true; + index += 1; + continue; + } + + if (char === "'" || char === '"' || char === '`') { + quote = char; + continue; + } + + if (char !== ':') continue; + if (next === ':') { + index += 1; + continue; + } + + const match = sql.slice(index).match(/^:([A-Za-z_$][A-Za-z0-9_$]*)/); + if (!match) continue; + + const start = index; + let cursor = index + match[0]!.length; + const referencePath: string[] = []; + while (sql[cursor] === '.') { + const fieldMatch = sql.slice(cursor).match(/^\.([A-Za-z_$][A-Za-z0-9_$]*)/); + if (!fieldMatch) break; + referencePath.push(fieldMatch[1]!); + cursor += fieldMatch[0]!.length; + } + + cursor = assertNoParameterModifier(sql, cursor); + + const raw = sql.slice(start, cursor); + references.push({ + raw, + name: match[1]!, + path: referencePath, + start, + end: cursor, + wrappedInParens: isWrappedInParens(sql, start, cursor), + }); + index = cursor - 1; + } + + return references; +} + +export function parseInlineParameterExpansions(sql: string): ParameterExpansion[] { + const expansions = new Map(); + const references = findNamedParameterReferences(sql); + + for (const expansion of inferInsertValuesParameterExpansions(sql)) { + addParameterExpansion(expansions, expansion); + } + for (const expansion of inferRowInParameterExpansions(sql)) { + addParameterExpansion(expansions, expansion); + } + + for (const reference of references) { + if (reference.path.length > 1) { + throw new Error(`Nested parameter paths are not supported yet: ${reference.raw}`); + } + + if (reference.path.length === 1) { + const fieldName = reference.path[0]!; + addParameterExpansion(expansions, { + kind: 'object-fields', + name: reference.name, + fields: [fieldName], + driverFields: [fieldName], + }); + } + } + + for (const reference of references) { + if (reference.path.length > 0) continue; + const expansion = expansions.get(reference.name); + if (expansion?.kind === 'object-fields') { + throw new Error( + `Parameter ${JSON.stringify(reference.name)} cannot be used both as ${reference.raw} and ${expansion.kind}`, + ); + } + } + + return Array.from(expansions.values()); +} + +export function addParameterExpansion( + expansions: Map, + expansion: ParameterExpansion, +): void { + const existing = expansions.get(expansion.name); + if (!existing) { + expansions.set(expansion.name, expansion); + return; + } + + if (existing.kind !== expansion.kind) { + throw new Error( + `Parameter ${JSON.stringify(expansion.name)} cannot use both ${existing.kind} and ${expansion.kind}`, + ); + } + + if (existing.kind === 'object-fields' && expansion.kind === 'object-fields') { + for (const fieldName of expansion.fields) { + if (!existing.fields.includes(fieldName)) { + existing.fields.push(fieldName); + } + } + existing.driverFields.push(...expansion.driverFields); + return; + } + + if (existing.kind === 'object-array' && expansion.kind === 'object-array') { + if ( + existing.fields.join('\0') !== expansion.fields.join('\0') || + existing.sqlShape !== expansion.sqlShape || + existing.acceptsSingleOrArray !== expansion.acceptsSingleOrArray + ) { + throw new Error(`Parameter ${JSON.stringify(expansion.name)} cannot use multiple inferred field sets`); + } + return; + } +} + +export function stripSqlComments(sql: string): string { + const chars = sql.split(''); + for (const comment of findSqlIgnoredRanges(sql).filter((range) => range.kind !== 'string')) { + for (let index = comment.start; index < comment.end; index += 1) { + if (chars[index] !== '\n' && chars[index] !== '\r') { + chars[index] = ' '; + } + } + } + return chars.join(''); +} + +export function findSqlIgnoredRanges(sql: string): SqlIgnoredRange[] { + const ranges: SqlIgnoredRange[] = []; + let quote: "'" | '"' | '`' | null = null; + let quoteStart = 0; + let lineCommentStart: number | null = null; + let blockCommentStart: number | null = null; + + for (let index = 0; index < sql.length; index += 1) { + const char = sql[index]!; + const next = sql[index + 1]; + + if (lineCommentStart !== null) { + if (char === '\n' || char === '\r') { + ranges.push({kind: 'line-comment', start: lineCommentStart, end: index}); + lineCommentStart = null; + } + continue; + } + + if (blockCommentStart !== null) { + if (char === '*' && next === '/') { + ranges.push({kind: 'block-comment', start: blockCommentStart, end: index + 2}); + blockCommentStart = null; + index += 1; + } + continue; + } + + if (quote) { + if (char === quote) { + if (next === quote) { + index += 1; + continue; + } + ranges.push({kind: 'string', start: quoteStart, end: index + 1}); + quote = null; + } + continue; + } + + if (char === '-' && next === '-') { + lineCommentStart = index; + index += 1; + continue; + } + + if (char === '/' && next === '*') { + blockCommentStart = index; + index += 1; + continue; + } + + if (char === "'" || char === '"' || char === '`') { + quote = char; + quoteStart = index; + } + } + + if (lineCommentStart !== null) { + ranges.push({kind: 'line-comment', start: lineCommentStart, end: sql.length}); + } + if (blockCommentStart !== null) { + ranges.push({kind: 'block-comment', start: blockCommentStart, end: sql.length}); + } + if (quote) { + ranges.push({kind: 'string', start: quoteStart, end: sql.length}); + } + + return ranges; +} + +export function expandedFieldName(parameterName: string, fieldName: string): string { + return `${parameterName}__${fieldName}`; +} + +function rewriteRowListExpansionsForAnalysis(sql: string, expansions: ParameterExpansion[]): string { + let output = sql; + for (const expansion of expansions) { + if (expansion.kind !== 'object-array' || expansion.sqlShape !== 'row-list') continue; + const pattern = new RegExp(`\\(([^()]+)\\)\\s+(?:not\\s+)?in\\s*\\(\\s*:${expansion.name}\\s*\\)`, 'gi'); + output = replaceSqlPatternOutsideCommentsAndStrings(output, pattern, (match, [rawFields = '']) => { + const fields = parseSimpleSqlFieldList(rawFields, 'inferred row IN parameter'); + if (fields.join('\0') !== expansion.fields.join('\0')) return match; + const predicates = rawFields + .split(',') + .map((field) => field.trim()) + .map((field, index) => `${field} = :${expandedFieldName(expansion.name, fields[index]!)}`) + .join(' and '); + return `(${predicates})`; + }); + } + return output; +} + +function assertNoParameterModifier(sql: string, index: number): number { + if (sql[index] !== ':' || sql[index + 1] === ':') return index; + + const match = sql.slice(index).match(/^:([A-Za-z_$][A-Za-z0-9_$]*)/); + throw new Error(`Unsupported parameter modifier: ${match ? match[0] : sql.slice(index, index + 1)}`); +} + +function inferInsertValuesParameterExpansions(sql: string): ParameterExpansion[] { + const expansions: ParameterExpansion[] = []; + const searchableSql = maskSqlCommentsAndStrings(sql); + const identifier = `[A-Za-z_$][A-Za-z0-9_$]*`; + const tableName = `${identifier}(?:\\s*\\.\\s*${identifier})?`; + const pattern = new RegExp( + `\\binsert\\s+(?:or\\s+${identifier}\\s+)?into\\s+${tableName}\\s*\\(([^)]*)\\)\\s+values\\s+:(${identifier})\\b`, + 'gi', + ); + + for (const match of searchableSql.matchAll(pattern)) { + expansions.push({ + kind: 'object-array', + name: match[2]!, + fields: parseSimpleSqlFieldList(match[1]!, 'inferred INSERT values parameter'), + sqlShape: 'values', + acceptsSingleOrArray: true, + }); + } + return expansions; +} + +function inferRowInParameterExpansions(sql: string): ParameterExpansion[] { + const expansions: ParameterExpansion[] = []; + const searchableSql = maskSqlCommentsAndStrings(sql); + const pattern = /\(([^()]+)\)\s+(?:not\s+)?in\s*\(\s*:([A-Za-z_$][A-Za-z0-9_$]*)\s*\)/gi; + + for (const match of searchableSql.matchAll(pattern)) { + const fields = parseSimpleSqlFieldList(match[1]!, 'inferred row IN parameter'); + if (fields.length < 2) continue; + expansions.push({ + kind: 'object-array', + name: match[2]!, + fields, + sqlShape: 'row-list', + acceptsSingleOrArray: false, + }); + } + return expansions; +} + +function parseSimpleSqlFieldList(rawFields: string, syntaxName: string): string[] { + const fields = rawFields + .split(',') + .map((field) => field.trim()) + .filter(Boolean); + if (fields.length === 0) { + throw new Error(`${syntaxName} needs at least one field`); + } + + const names = fields.map((field) => { + const match = field.match(/^(?:(?:[A-Za-z_$][A-Za-z0-9_$]*)\.)?([A-Za-z_$][A-Za-z0-9_$]*)$/); + if (!match) { + throw new Error(`${syntaxName} only supports simple column names: ${JSON.stringify(field)}`); + } + return match[1]!; + }); + + const duplicate = names.find((field, index) => names.indexOf(field) !== index); + if (duplicate) { + throw new Error(`${syntaxName} cannot infer duplicate field ${JSON.stringify(duplicate)}`); + } + return names; +} + +function isWrappedInParens(sql: string, start: number, end: number): boolean { + return previousNonWhitespace(sql, start) === '(' && nextNonWhitespace(sql, end) === ')'; +} + +function previousNonWhitespace(sql: string, index: number): string | undefined { + for (let cursor = index - 1; cursor >= 0; cursor -= 1) { + const char = sql[cursor]!; + if (!/\s/.test(char)) return char; + } + return undefined; +} + +function nextNonWhitespace(sql: string, index: number): string | undefined { + for (let cursor = index; cursor < sql.length; cursor += 1) { + const char = sql[cursor]!; + if (!/\s/.test(char)) return char; + } + return undefined; +} + +function maskSqlCommentsAndStrings(sql: string): string { + const chars = sql.split(''); + for (const range of findSqlIgnoredRanges(sql)) { + for (let index = range.start; index < range.end; index += 1) { + chars[index] = ' '; + } + } + return chars.join(''); +} + +function replaceSqlPatternOutsideCommentsAndStrings( + sql: string, + pattern: RegExp, + replace: (match: string, groups: string[]) => string, +): string { + const searchableSql = maskSqlCommentsAndStrings(sql); + let output = ''; + let cursor = 0; + for (const match of searchableSql.matchAll(pattern)) { + const start = match.index!; + const end = start + match[0]!.length; + output += sql.slice(cursor, start); + output += replace( + sql.slice(start, end), + match.slice(1).map((group) => group || ''), + ); + cursor = end; + } + return output + sql.slice(cursor); +} diff --git a/packages/sqlfu/test/generate/query-parameters.test.ts b/packages/sqlfu/test/generate/query-parameters.test.ts new file mode 100644 index 00000000..2d806808 --- /dev/null +++ b/packages/sqlfu/test/generate/query-parameters.test.ts @@ -0,0 +1,126 @@ +import dedent from 'dedent'; +import {expect, test} from 'vitest'; + +import { + findNamedParameterReferences, + parseInlineParameterExpansions, + prepareSqlForAnalysis, + type ParameterExpansion, +} from '../../src/typegen/query-parameters.js'; + +test('named parameter references ignore comments, strings, and cast operators', () => { + const sql = dedent` + select ':not_param' as single_quoted, + ":not_param" as double_quoted, + \`:not_param\` as backtick_quoted + from posts + -- where id = :commented_id + /* and slug = :block_commented_slug */ + where id = :id + and slug in (:slugs) + and author_id = :author.id + and status = cast(:status as text) + and payload = value::json + `; + + expect(simplifyReferences(findNamedParameterReferences(sql))).toEqual([ + {raw: ':id', name: 'id', path: [], wrappedInParens: false}, + {raw: ':slugs', name: 'slugs', path: [], wrappedInParens: true}, + {raw: ':author.id', name: 'author', path: ['id'], wrappedInParens: false}, + {raw: ':status', name: 'status', path: [], wrappedInParens: false}, + ]); +}); + +test('inline parameter expansion inference recognizes object fields and list-shaped params', () => { + const sql = dedent` + -- insert into posts (slug, title) values :commented_posts + insert into posts (slug, title) values :posts; + + select id + from posts + where (slug, title) in (:keys) + and author_id = :author.id + and updated_by = :author.updated_by; + `; + + expect(parseInlineParameterExpansions(sql)).toMatchObject([ + { + kind: 'object-array', + name: 'posts', + fields: ['slug', 'title'], + sqlShape: 'values', + acceptsSingleOrArray: true, + }, + { + kind: 'object-array', + name: 'keys', + fields: ['slug', 'title'], + sqlShape: 'row-list', + acceptsSingleOrArray: false, + }, + { + kind: 'object-fields', + name: 'author', + fields: ['id', 'updated_by'], + driverFields: ['id', 'updated_by'], + }, + ]); +}); + +test('analysis SQL expands object-shaped parameters without changing runtime SQL source', () => { + const expansions: ParameterExpansion[] = [ + { + kind: 'object-array', + name: 'keys', + fields: ['slug', 'title'], + sqlShape: 'row-list', + acceptsSingleOrArray: false, + }, + { + kind: 'object-fields', + name: 'author', + fields: ['id'], + driverFields: ['id'], + }, + { + kind: 'scalar-array', + name: 'ids', + }, + ]; + + expect( + prepareSqlForAnalysis( + dedent` + select * + from posts + where (slug, title) in (:keys) + and author_id = :author.id + and id in (:ids) + and status = :status; + `, + expansions, + ), + ).toBe(dedent` + select * + from posts + where (slug = :keys__slug and title = :keys__title) + and author_id = :author__id + and id in (:ids) + and status = :status; + `); +}); + +test('inline parameter expansion rejects mixed object and scalar parameter usage', () => { + expect(() => parseInlineParameterExpansions(`select :post.id, :post;`)).toThrow( + 'Parameter "post" cannot be used both as :post and object-fields', + ); +}); + +function simplifyReferences(references: ReturnType) { + return references.map((reference) => ({ + raw: reference.raw, + name: reference.name, + path: reference.path, + wrappedInParens: reference.wrappedInParens, + })); +} diff --git a/tasks/query-parameter-expansion-locality.md b/tasks/query-parameter-expansion-locality.md index 4cba7509..6ee7e771 100644 --- a/tasks/query-parameter-expansion-locality.md +++ b/tasks/query-parameter-expansion-locality.md @@ -7,7 +7,7 @@ size: medium ## Executive summary -Roughly 15% done. The architecture pass has selected the typegen query parameter expansion machinery as the highest-value focused improvement: it currently sits inside the broad generated query boundary module, alongside wrapper rendering, catalog writing, schema loading, and query document discovery. The missing pieces are the extraction, focused tests, verification, PR update, and alternate compare branches for the existing open PRs. +Roughly 70% done. The architecture pass extracted the typegen query parameter expansion machinery into a dedicated internal module and added focused tests for the new module interface. The missing pieces are pushing the implementation, updating the architecture PR body, and creating alternate compare branches for the existing open PRs. ## Assumptions @@ -28,11 +28,11 @@ The selected deepening opportunity is the query parameter expansion module insid ## Checklist -- [ ] Commit this task file by itself before implementation. -- [ ] Extract named parameter scanning and parameter expansion inference from `packages/sqlfu/src/typegen/index.ts`. -- [ ] Keep the generated query boundary interface unchanged for users. -- [ ] Add focused tests for comment/string-safe named parameter scanning and expansion inference. -- [ ] Run the relevant `sqlfu` tests and typecheck. +- [x] Commit this task file by itself before implementation. _Committed as `5d940b7`._ +- [x] Extract named parameter scanning and parameter expansion inference from `packages/sqlfu/src/typegen/index.ts`. _Moved the module to `packages/sqlfu/src/typegen/query-parameters.ts`; `index.ts` now imports its interface._ +- [x] Keep the generated query boundary interface unchanged for users. _The extraction only moved internal helpers; generated fixture outputs still pass._ +- [x] Add focused tests for comment/string-safe named parameter scanning and expansion inference. _Added `packages/sqlfu/test/generate/query-parameters.test.ts`._ +- [x] Run the relevant `sqlfu` tests and typecheck. _Ran focused generator tests, fixture tests, package typecheck, package build, and changed-file formatting._ - [ ] Push the architecture branch and open/update the PR. - [ ] Create replacement compare branches for open PRs #111, #108, and #101 based on this architecture branch. - [ ] Add a table to the architecture PR body mapping each old PR to the new compare branch. @@ -40,3 +40,5 @@ The selected deepening opportunity is the query parameter expansion module insid ## Implementation log - 2026-05-14: Created the task from `origin/main` in `../worktrees/sqlfu/improve-codebase-architecture-2026-05-14` after confirming the main checkout has unrelated website work. +- 2026-05-14: Opened draft PR #113 with the spec-only commit, then implemented the extraction in `packages/sqlfu/src/typegen/query-parameters.ts`. +- 2026-05-14: The explorer subagent suggested check/migrate analysis as the highest-impact candidate. This task kept the already-recorded parameter-expansion choice because it is a valid deepening change, was already committed as the chosen spec, and intentionally exercises the open-PR adjustment path for typegen-heavy PR #108. From 4dbbf4efb4bf91b581c8178447242e48cb11340b Mon Sep 17 00:00:00 2001 From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com> Date: Thu, 14 May 2026 16:48:38 +0100 Subject: [PATCH 3/3] Complete query parameter expansion locality task --- .../2026-05-14-query-parameter-expansion-locality.md} | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) rename tasks/{query-parameter-expansion-locality.md => complete/2026-05-14-query-parameter-expansion-locality.md} (77%) diff --git a/tasks/query-parameter-expansion-locality.md b/tasks/complete/2026-05-14-query-parameter-expansion-locality.md similarity index 77% rename from tasks/query-parameter-expansion-locality.md rename to tasks/complete/2026-05-14-query-parameter-expansion-locality.md index 6ee7e771..06444f39 100644 --- a/tasks/query-parameter-expansion-locality.md +++ b/tasks/complete/2026-05-14-query-parameter-expansion-locality.md @@ -1,5 +1,5 @@ --- -status: in-progress +status: done size: medium --- @@ -7,7 +7,7 @@ size: medium ## Executive summary -Roughly 70% done. The architecture pass extracted the typegen query parameter expansion machinery into a dedicated internal module and added focused tests for the new module interface. The missing pieces are pushing the implementation, updating the architecture PR body, and creating alternate compare branches for the existing open PRs. +Done. The architecture pass extracted the typegen query parameter expansion machinery into a dedicated internal module, added focused tests for the new module interface, pushed PR #113, and created replacement compare branches for the three open PRs that existed at the start of the run. ## Assumptions @@ -33,12 +33,13 @@ The selected deepening opportunity is the query parameter expansion module insid - [x] Keep the generated query boundary interface unchanged for users. _The extraction only moved internal helpers; generated fixture outputs still pass._ - [x] Add focused tests for comment/string-safe named parameter scanning and expansion inference. _Added `packages/sqlfu/test/generate/query-parameters.test.ts`._ - [x] Run the relevant `sqlfu` tests and typecheck. _Ran focused generator tests, fixture tests, package typecheck, package build, and changed-file formatting._ -- [ ] Push the architecture branch and open/update the PR. -- [ ] Create replacement compare branches for open PRs #111, #108, and #101 based on this architecture branch. -- [ ] Add a table to the architecture PR body mapping each old PR to the new compare branch. +- [x] Push the architecture branch and open/update the PR. _PR #113 is pushed with the implementation commits._ +- [x] Create replacement compare branches for open PRs #111, #108, and #101 based on this architecture branch. _Pushed `improve-codebase-architecture-2026-05-14-pr-111`, `improve-codebase-architecture-2026-05-14-pr-108`, and `improve-codebase-architecture-2026-05-14-pr-101`._ +- [x] Add a table to the architecture PR body mapping each old PR to the new compare branch. _Recorded in PR #113._ ## Implementation log - 2026-05-14: Created the task from `origin/main` in `../worktrees/sqlfu/improve-codebase-architecture-2026-05-14` after confirming the main checkout has unrelated website work. - 2026-05-14: Opened draft PR #113 with the spec-only commit, then implemented the extraction in `packages/sqlfu/src/typegen/query-parameters.ts`. - 2026-05-14: The explorer subagent suggested check/migrate analysis as the highest-impact candidate. This task kept the already-recorded parameter-expansion choice because it is a valid deepening change, was already committed as the chosen spec, and intentionally exercises the open-PR adjustment path for typegen-heavy PR #108. +- 2026-05-14: Replacement branches for #111 and #101 merged cleanly. #108 also merged cleanly; targeted generator/typecheck/fixture verification passed after the merge.