Skip to content

Use sqlite3-parser for schemadiff reference analysis#111

Draft
mmkal wants to merge 3 commits into
mainfrom
issue-110-sqlite3-parser-schemadiff
Draft

Use sqlite3-parser for schemadiff reference analysis#111
mmkal wants to merge 3 commits into
mainfrom
issue-110-sqlite3-parser-schemadiff

Conversation

@mmkal
Copy link
Copy Markdown
Collaborator

@mmkal mmkal commented May 12, 2026

Summary

This PR adds sqlite3-parser as a normal sqlfu dependency and uses it for SQLite schemadiff reference analysis.

The merge effect is deliberately narrow: sqlfu now parses stored SQLite SQL bodies for view, trigger, and check(...) references, then translates the AST into sqlfu-owned facts (referencedTables, referencedColumns) before planner code sees it. SQLite catalog inspection remains the authority for tables, columns, indexes, keys, generated columns, and similar schema metadata.

Closes #110

Behavior Improvements

The justification here is concrete examples rather than a line-count win. analysis.ts gets smaller, but the new collector is intentionally explicit and adds code overall.

CTE shadowing no longer creates a false view blocker

Before, token scanning saw person and nickname in this view and dropped/recreated it when person.nickname was removed:

create view cte_person_names as
with person as (select 'display' as nickname)
select nickname from person;

Now the parser-backed collector understands that person is a CTE name in that scope, so the diff can stay at:

alter table person drop column nickname;

Trigger writes to another table no longer block the subject table drop

Before, this trigger on person was treated as a blocker for dropping person.nickname because token scanning saw the audit_log(nickname) insert column:

create trigger person_log after update on person begin
  insert into audit_log(nickname) values ('changed');
end;

Now trigger-subject column references are distinguished from target-table insert columns, so this also stays a direct column drop.

Implementation Notes

  • Added sqlite3-parser@0.7.1 to packages/sqlfu dependencies.
  • Added packages/sqlfu/src/schemadiff/sqlite/references.ts as the only module that talks to third-party AST nodes.
  • Kept analysis.ts returning the existing SqliteDependencyFact shape.
  • Replaced the old check-constraint splitter with parsed CreateTableStmt check-expression collection, with the previous heuristic retained only as a parse-failure fallback.
  • Left formatter, TypeSQL, and partial-index where parsing out of scope.
  • Moved the task file to tasks/complete/2026-05-12-sqlite3-parser-schemadiff.md.

Size

After pnpm --filter sqlfu build, cd packages/sqlfu && npm pack --dry-run --json reports:

  • packed: 241,990 bytes
  • unpacked: 1,005,020 bytes
  • files: 181

The external dependency is not bundled into the sqlfu tarball; users will install the separate sqlite3-parser package as a runtime dependency.

Verification

  • pnpm --filter sqlfu test --run test/schemadiff
  • pnpm --filter sqlfu typecheck
  • pnpm --filter sqlfu build
  • cd packages/sqlfu && npm pack --dry-run --json
Package size — packed 239.6 kB (+3.2 kB, +1.3%)

Package size

main this PR Δ
packed 236.5 kB 239.6 kB +1.3%
unpacked 971.3 kB 995.8 kB +2.5%
files 181 183 +2

dist/vendor/*.js bundles

main this PR Δ
vendor/sha256.js 4.3 kB 4.3 kB 0
vendor/sql-formatter/*.js 58.3 kB 58.3 kB 0
vendor/sqlfu-sqlite-parser/*.js 17.2 kB 17.2 kB 0
vendor/standard-schema/*.js 2.8 kB 2.8 kB 0
vendor/typesql/*.js 134.6 kB 134.6 kB 0

Measured with npm pack --dry-run --json on sqlfu (0.0.3-7 on main vs 0.0.3-7 on this PR).

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 12, 2026

Open in StackBlitz

npm i https://pkg.pr.new/sqlfu@111

commit: 9d8332c

mmkal added a commit that referenced this pull request May 14, 2026
## Summary

This architecture pass extracts query parameter expansion out of the
broad generated query boundary module. SQL parameter scanning, inline
expansion inference, analysis-SQL rewriting, and reusable replacement
helpers now live behind `packages/sqlfu/src/typegen/query-parameters.ts`
instead of being embedded in `typegen/index.ts`.

The generated query boundary stays user-compatible; this is an internal
locality/depth change with focused tests on the new module interface.

## Why

`packages/sqlfu/src/typegen/index.ts` mixed generated wrapper rendering,
query document loading, validator emission, catalog writing, schema
materialization, and named-parameter expansion. Parameter expansion has
its own invariants: comment/string-safe scanning, object-field params,
list params, row-list params, and generated runtime SQL. Moving those
behind one module gives future changes better locality and lets tests
exercise that behavior without going through full fixture generation.

## Verification

Architecture branch:

- `pnpm exec oxfmt --check packages/sqlfu/src/typegen/index.ts
packages/sqlfu/src/typegen/query-parameters.ts
packages/sqlfu/test/generate/query-parameters.test.ts`
- `pnpm --filter sqlfu typecheck`
- `pnpm --filter sqlfu build`
- `pnpm --filter sqlfu test --run test/generate/query-parameters.test.ts
test/generate/runtime.test.ts`
- `pnpm --filter sqlfu test --run test/generate/fixtures.test.ts`

Replacement branches:

- #111 branch: `pnpm --filter sqlfu typecheck`; `pnpm --filter sqlfu
test --run test/schemadiff/fixtures.test.ts
test/schemadiff/plumbing.test.ts test/generate/query-parameters.test.ts`
- #108 branch: `pnpm --filter sqlfu typecheck`; `pnpm --filter sqlfu
test --run test/generate/query-parameters.test.ts
test/generate/runtime.test.ts`; `pnpm --filter sqlfu test --run
test/generate/fixtures.test.ts`
- #101 branch: `pnpm --filter sqlfu typecheck`; `pnpm --filter @sqlfu/ui
typecheck`; `pnpm --filter sqlfu test --run
test/adapters/sqlite-wasm.test.ts test/sqlite-text.test.ts
test/generate/query-parameters.test.ts`; `pnpm --filter @sqlfu/ui exec
vitest run src/demo/browser-host.test.ts`

## Open PR replacement branches

| Existing PR | Existing branch | Replacement compare |
| --- | --- | --- |
| [#111](#111) |
`issue-110-sqlite3-parser-schemadiff` |
[`improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-111`](improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-111)
|
| [#108](#108) | `typegen-casing` |
[`improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-108`](improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-108)
|
| [#101](#101) |
`sql-runner-named-params-2026-05-12` |
[`improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-101`](improve-codebase-architecture-2026-05-14...improve-codebase-architecture-2026-05-14-pr-101)
|

<!-- package-size:start -->
<details>
<summary>Package size — packed 234.4 kB (+307 B, +0.1%)</summary>

## Package size

|  | main | this PR | Δ |
| - | - | - | - |
| packed | 234.1 kB | 234.4 kB | +0.1% |
| unpacked | 958.1 kB | 960.5 kB | +0.3% |
| files | 179 | 181 | +2 |

### `dist/vendor/*.js` bundles

|  | main | this PR | Δ |
| - | - | - | - |
| `vendor/sha256.js` | 4.3 kB | 4.3 kB | 0 |
| `vendor/sql-formatter/*.js` | 58.3 kB | 58.3 kB | 0 |
| `vendor/sqlfu-sqlite-parser/*.js` | 17.2 kB | 17.2 kB | 0 |
| `vendor/standard-schema/*.js` | 2.8 kB | 2.8 kB | 0 |
| `vendor/typesql/*.js` | 134.6 kB | 134.6 kB | 0 |

_Measured with `npm pack --dry-run --json` on `sqlfu` (0.0.3-7 on main
vs 0.0.3-7 on this PR)._

</details>
<!-- package-size:end -->

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Mostly an internal refactor that moves existing SQL parameter
parsing/rewriting logic into `query-parameters.ts` with new unit tests;
risk is limited to potential regressions in typegen parameter inference
and analysis-SQL rewriting.
> 
> **Overview**
> Extracts the query parameter parsing/expansion machinery
(named-parameter scanning, inline expansion inference, comment/string
masking, and analysis-SQL rewriting) out of `typegen/index.ts` into a
new internal module `typegen/query-parameters.ts`, with `index.ts` now
importing and using that API.
> 
> Adds focused Vitest coverage for `findNamedParameterReferences`,
`parseInlineParameterExpansions`, and `prepareSqlForAnalysis` to ensure
parameters are ignored inside comments/strings and that
object/list-shaped params expand correctly for analysis without changing
the runtime SQL source.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
4dbbf4e. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
mmkal added a commit that referenced this pull request May 16, 2026
## Summary

Deepens runtime parameter binding into a driver-neutral Module behind
the Adapter seam.

- Adds `packages/sqlfu/src/sql-params.ts` for named SQL parameter
scanning and driver bind-shape projection.
- Moves D1, Durable Objects, Expo SQLite, Turso serverless, sqlite-wasm,
and Postgres onto that Module.
- Deletes the old sqlite-text named-param helper and the Postgres
private `?` to `$N` scanner, so Postgres named params now bind directly
to `$1`, `$2`, etc. while preserving dollar-quoted SQL bodies.
- Removes the old root export for `rewriteNamedParamsToPositional`; this
was adapter plumbing, not product surface.

## Design Choice

The `grill-you` pass challenged whether Postgres should keep its private
scanner. The decision was to use one extended scanner for binding,
including Postgres dollar-quoted strings, because two scanners over the
same authored SQL were the source of the architectural wart.

## Checks

Passed:

- `pnpm --filter sqlfu exec vitest run test/sql-params.test.ts
test/adapters/pg.test.ts test/sqlite-text.test.ts`
- `pnpm --filter sqlfu exec vitest run test/adapters/d1.test.ts
test/adapters/durable-object.test.ts test/adapters/expo-sqlite.test.ts
test/adapters/sqlite-wasm.test.ts`
- `pnpm --filter sqlfu exec vitest run
test/adapters/turso-remote.test.ts test/adapters/turso-database.test.ts`
- `pnpm --filter sqlfu typecheck`
- `pnpm --filter @sqlfu/ui build`
- `pnpm --filter sqlfu exec vitest run test/resolve-sqlfu-ui.test.ts`
- `git diff --check`

Full `pnpm --filter sqlfu test` still fails
`test/import-surface.test.ts` because the existing vendored TypeSQL
bundle for `sqlfu/analyze` imports `node:sqlite`. That is outside this
PR; the initial full run also needed `@sqlfu/ui` built before
`resolve-sqlfu-ui` passed.

## Replacement Compare Branches

| Existing PR | Replacement compare | Strategy |
| --- | --- | --- |
| [#124
`bedtime/2026-05-15-default-db-gitignore`](#124)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-124)
| clean merge |
| [#123
`bedtime/2026-05-15-pg-docs-followup`](#123)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-123)
| clean merge |
| [#122
`bedtime/2026-05-15-db-base-directory`](#122)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-122)
| clean merge |
| [#121
`bedtime/2026-05-15-improve-docs`](#121)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-121)
| clean merge |
| [#120
`bedtime/2026-05-15-landing-trace`](#120)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-120)
| clean merge |
| [#119
`bedtime/2026-05-15-cleanup-tasks`](#119)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-119)
| clean merge |
| [#117
`bedtime/2026-05-14-query-identity-manifest`](#117)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-117)
| clean merge |
| [#114
`bedtime/2026-05-14-generate-preflight`](#114)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-114)
| clean merge |
| [#111
`issue-110-sqlite3-parser-schemadiff`](#111)
|
[comparison](bedtime/2026-05-15-architecture...bedtime/2026-05-15-architecture-pr-111)
| clean merge |

Replacement checks: all replacement branches passed `git diff --check`;
targeted tests/builds are recorded in
`tasks/architecture-2026-05-15.md`.

<!-- package-size:start -->
<details>
<summary>Package size — packed 236.8 kB (+259 B, +0.1%)</summary>

## Package size

|  | main | this PR | Δ |
| - | - | - | - |
| packed | 236.5 kB | 236.8 kB | +0.1% |
| unpacked | 971.5 kB | 974.5 kB | +0.3% |
| files | 181 | 183 | +2 |

### `dist/vendor/*.js` bundles

|  | main | this PR | Δ |
| - | - | - | - |
| `vendor/sha256.js` | 4.3 kB | 4.3 kB | 0 |
| `vendor/sql-formatter/*.js` | 58.3 kB | 58.3 kB | 0 |
| `vendor/sqlfu-sqlite-parser/*.js` | 17.2 kB | 17.2 kB | 0 |
| `vendor/standard-schema/*.js` | 2.8 kB | 2.8 kB | 0 |
| `vendor/typesql/*.js` | 134.6 kB | 134.6 kB | 0 |

_Measured with `npm pack --dry-run --json` on `sqlfu` (0.0.3-7 on main
vs 0.0.3-7 on this PR)._

</details>
<!-- package-size:end -->

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches SQL parameter rewriting/binding used by multiple runtime
adapters (including Postgres), so subtle placeholder-scanning edge cases
could change query behavior. Added targeted unit tests for Postgres and
the new binder to reduce regression risk.
> 
> **Overview**
> Moves SQL placeholder scanning and parameter binding into a new
driver-neutral `sql-params.ts`, supporting `?`-style positional,
Postgres `$N` placeholders, and sqlite-wasm’s prefixed named binds.
> 
> Updates D1, Durable Objects, Expo SQLite, Turso serverless,
sqlite-wasm, and the Postgres adapter to use the shared binder; Postgres
drops its custom `?`→`$N` conversion and now binds named params directly
while preserving dollar-quoted strings, casts, and JSON `?` operators.
> 
> Removes the adapter-plumbing export `rewriteNamedParamsToPositional`
(and its tests) from `sqlite-text.ts`/root `index.ts`, and adds new unit
tests covering the shared binder plus Postgres binding behavior.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
8780812. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

## Review Follow-up

Independent review found two Postgres scanner gaps after the first push:
`:param::type` casts and JSONB `?` / `?|` / `?&` operators. Both are
fixed in `8780812` with regression tests in `test/sql-params.test.ts`
and `test/adapters/pg.test.ts`. Replacement compare branches were
refreshed by merging the updated architecture head and all replacement
branches still pass `git diff --check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Evaluate sqlite3-parser for SQLite AST-backed analysis

1 participant