Skip to content

Commit 79dc271

Browse files
authored
feat(vnext): add bounded node-sql-parser adapter (#177)
## Summary - add an internal normalized node-sql-parser adapter for PostgreSQL, BigQuery, and DuckDB compatibility evidence - pin node-sql-parser 5.4.0 and load only dialect-specific builds through a race-safe pure-Node loader - keep all successful evidence non-conformant, retain ASTs privately, and fail closed in browser/worker/DOM-shim realms pending dedicated-worker support - document the evidence, execution, resource, packaging, and global-cleanup boundaries in ADR 0003 - add adversarial unit/browser coverage and validated warm parser benchmarks ## Correctness policy - PostgreSQL/BigQuery acceptance: parsed/compatibility with partial-artifact - PostgreSQL/BigQuery rejection: unsupported/uncovered-construct, never invalid - DuckDB acceptance: parsed/compatibility with dialect-compatibility + partial-artifact - DuckDB rejection: unsupported/compatibility-rejected - Dremio: no adapter ## Validation - 971 tests passed, 1 expected failure - changed coverage: 99.13% statements, 99.35% branches, 100% functions, 99.12% lines - full strict typecheck and non-mutating lint - Chromium browser suite - isolated package smoke and demo build - test integrity check - preflight-validated parser benchmark - two independent adversarial review loops approved the final exact tree <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Add an internal `node-sql-parser` adapter for PostgreSQL and BigQuery with a DuckDB compatibility path that returns small, private artifacts under strict bounds and pure-Node loading. Also fixes and documents a test spelling false positive, and keeps the CI report spelling-clean. - **New Features** - Evidence policy: PostgreSQL/BigQuery accept -> parsed/compatibility (partial); reject -> unsupported/uncovered-construct. DuckDB uses the PostgreSQL build: accept -> parsed/compatibility (dialect-compatibility, partial); reject -> unsupported/compatibility-rejected. - Bounded artifacts: only statement kind + full range; backend ASTs kept private. - Resource/placement: 16 KiB statement limit; synchronous parse; cancellation checked before/after load and after parse; not wired to sessions. - Execution realm safety: pure Node only with race-safe loader and global cleanup; fails closed in window/worker/DOM-shim realms. - Docs and tests: added ADR 0003, linked ADR 0002, and a vNext adapter doc; Node and browser tests plus a warm parser benchmark and `bench:parser-adapter` script; fixed spelling CI false positive and kept `state/artifacts/reports/pr-177-spelling-ci.md` spelling-clean. - **Dependencies** - Pin `node-sql-parser` to `5.4.0` and load only `build/postgresql.js` and `build/bigquery.js`. <sup>Written for commit 2543624. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/177?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent 9e9e17e commit 79dc271

11 files changed

Lines changed: 2483 additions & 2 deletions

docs/adr/0002-normalized-syntax-contract.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ request locations, retain backend data privately, and normalize every boundary
179179
before constructing these results. Cache/session wiring follows only after the
180180
adapter passes the contract suite.
181181

182+
[ADR 0003](./0003-node-sql-parser-adapter.md) applies this contract
183+
conservatively: PostgreSQL, BigQuery, and DuckDB acceptance is compatibility
184+
evidence with explicit limitations; rejection is unsupported rather than
185+
invalid; and session wiring waits for an explicit
186+
synchronous-execution/worker decision.
187+
182188
## Non-goals
183189

184190
This decision does not define:
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
# ADR 0003: Internal node-sql-parser Adapter
2+
3+
Status: accepted
4+
Date: 2026-07-25
5+
6+
## Context
7+
8+
ADR 0002 defines an internal normalized syntax contract, but it deliberately
9+
does not decide how a concrete parser earns authority. The first adapter uses
10+
`node-sql-parser` because the dependency is already present and can provide a
11+
useful local AST in Node or an isolated worker. It must not turn the
12+
dependency's incomplete grammar,
13+
partial locations, synchronous execution, or packaging behavior into broader
14+
vNext guarantees.
15+
16+
The installed `node-sql-parser` 5.4.0 package provides separate CommonJS
17+
bundles for PostgreSQL and BigQuery. The complete bundle is approximately
18+
2.5 MB uncompressed and 428 KiB compressed, while the PostgreSQL and BigQuery
19+
builds are approximately 59 KiB and 42 KiB compressed respectively. Loading
20+
the complete bundle would make consumers pay for unrelated grammars.
21+
22+
The target-named grammars are not authoritative validators for their database
23+
engines. In local probes, the PostgreSQL build rejected valid `MERGE`,
24+
`FETCH FIRST`, `INSERT ... DEFAULT VALUES`, identity-column, and
25+
`FOR UPDATE SKIP LOCKED` statements. The BigQuery build rejected valid
26+
`QUALIFY`, `MERGE`, and `DECLARE` statements. A parse rejection therefore
27+
cannot establish that target-dialect SQL is invalid.
28+
29+
The parser also runs synchronously. An `AbortSignal` can prevent work before
30+
the parse begins and suppress publication after it finishes, but it cannot
31+
interrupt `astify` while JavaScript is blocked.
32+
33+
## Decision
34+
35+
The adapter remains internal and is not exported from `/vnext`. This change
36+
does not connect it to document sessions, caches, diagnostics, completion, or
37+
any other feature. Session wiring requires a separate decision about worker
38+
isolation and cancellation.
39+
40+
### Dependency and loading
41+
42+
Pin `node-sql-parser` exactly to version `5.4.0`. The adapter depends on deep
43+
paths, CommonJS interop, AST shapes, and grammar behavior that are not stable
44+
enough for an unconstrained dependency range.
45+
46+
Load only these extension-qualified paths:
47+
48+
```text
49+
node-sql-parser/build/postgresql.js
50+
node-sql-parser/build/bigquery.js
51+
```
52+
53+
After dynamically acquiring `createRequire` from `node:module`, the adapter
54+
revalidates the realm and synchronously requires the selected build. Module
55+
evaluation and global restoration therefore run in one JavaScript stack with
56+
no alias-mutation gap. The adapter does not load the all-dialect entry point
57+
and does not pass a `database` option to a dialect-specific build.
58+
59+
Node ESM exposes these CommonJS builds through `default`/`module.exports` even
60+
though the declarations suggest a named `Parser` export. The module boundary
61+
is decoded from `unknown` and accepts only a runtime-validated constructor and
62+
`astify` method. No unchecked assertion compensates for the declaration/runtime
63+
mismatch.
64+
65+
The parser receives fixed options:
66+
67+
```ts
68+
{
69+
trimQuery: false,
70+
parseOptions: {
71+
includeLocations: true,
72+
},
73+
}
74+
```
75+
76+
Disabling trimming is required to keep every backend offset relative to the
77+
exact untrimmed statement slice. Location inclusion is requested for future
78+
private semantic decoding, but location availability remains per node and is
79+
not promoted to a dialect-wide capability.
80+
81+
The distributed UMD-style builds can assign `NodeSQLParser` and, in some
82+
environments, `global` on the global object while loading. The adapter refuses
83+
to load them whenever `window` or `self` exists, or `global` does not resolve
84+
exactly to `globalThis`. Only pure Node is currently eligible. This prevents
85+
global collisions, including DOM shims in Node, and accidental synchronous
86+
main-thread parsing. The Node loader captures the existing own-property
87+
descriptors for the affected names and restores them synchronously after module
88+
evaluation. A module that cannot be loaded and cleaned up safely permanently
89+
poisons loading and returns a non-retryable backend failure.
90+
91+
### Evidence policy
92+
93+
PostgreSQL and BigQuery acceptance is positive-only compatibility evidence. A
94+
successful target-specific parse produces `parsed/compatibility` with the
95+
`partial-artifact` limitation. It means only that the target-named backend
96+
produced a bounded normalized artifact. The adapter does not create a
97+
conformance identity because the grammar also accepts constructs that the
98+
target engines reject.
99+
100+
Every PostgreSQL or BigQuery parse rejection produces:
101+
102+
```text
103+
unsupported / uncovered-construct
104+
```
105+
106+
It never produces `invalid`, even for input that appears obviously malformed.
107+
The same parser error is indistinguishable from rejection of valid syntax that
108+
the dependency does not implement. This adapter therefore creates no
109+
authoritative syntax diagnostics.
110+
111+
DuckDB uses the PostgreSQL build only as a compatibility parser:
112+
113+
- Acceptance produces `parsed/compatibility` with the
114+
`dialect-compatibility` and `partial-artifact` limitations.
115+
- Rejection produces `unsupported/compatibility-rejected`.
116+
- The adapter performs no rewrites, success-without-AST shortcuts, bracket
117+
quoting, comment stripping, or offset repair.
118+
119+
Dremio has no node-sql-parser adapter. When a coordinator is added later, it
120+
will classify Dremio as `unavailable/dialect-not-supported` rather than silently
121+
selecting another grammar.
122+
123+
Unexpected backend exceptions are failures, not syntax evidence. Invalid
124+
module shapes or malformed AST roots are `failed/malformed-output`; other
125+
unexpected parser exceptions are `failed/backend-failure`. Raw exceptions and
126+
backend messages do not cross the normalized boundary.
127+
128+
### Input and artifacts
129+
130+
The adapter imposes a 16 KiB statement limit, below the syntax contract's
131+
general 1 MiB ceiling. A larger statement returns
132+
`unsupported/resource-limit` before importing or invoking the backend.
133+
This conservative limit bounds work and temporary parser memory, but it does
134+
not satisfy the provisional active-statement latency envelope: warm local
135+
16 KiB parses already exceed that target. Placement measurements must set a
136+
lower interactive limit or move parsing off the main thread.
137+
138+
The backend receives only the exact, untrimmed, code-bearing statement source.
139+
It receives no terminator, document coordinates, editor state, catalog, cursor,
140+
or host context.
141+
142+
A successful result must contain exactly one object root with a string `type`.
143+
A direct object or one-element array is accepted. Zero or multiple roots are
144+
`unsupported/uncovered-construct`; malformed roots are
145+
`failed/malformed-output`.
146+
147+
The public normalized artifact exposes only its closed statement kind and full
148+
statement-relative range. The backend AST is retained in a module-private
149+
`WeakMap` keyed by the authenticated artifact. It is neither enumerable nor
150+
returned through the syntax contract. Future relation extraction must decode
151+
and validate backend nodes inside the adapter boundary rather than exposing
152+
the raw AST to core features.
153+
154+
### Cancellation and execution placement
155+
156+
The callback checks cancellation before loading, after loading, and after
157+
parsing. These checks preserve the syntax runner's request-lifecycle behavior,
158+
but they cannot preempt synchronous parser execution.
159+
160+
The adapter must not be wired into interactive session requests until a
161+
follow-up decision records one of:
162+
163+
- Worker/process isolation with enforceable wall-clock and memory limits, or
164+
- Measured main-thread execution with an accepted adversarial-input risk and
165+
explicit scheduling policy.
166+
167+
That decision must include browser measurements, cancellation behavior,
168+
worker/module failure recovery, and the effect of many mounted editors.
169+
170+
The current production loader supports pure Node only. A future browser
171+
integration must invoke parsing from a dedicated worker whose global object is
172+
not shared with application code, the legacy parser, or another installed copy.
173+
The unsupported-realm rejection remains until that isolated execution path
174+
exists.
175+
176+
## Consequences
177+
178+
- Local parsing can be evaluated without exposing a backend AST or
179+
committing the language service to one parser.
180+
- PostgreSQL and BigQuery false negatives degrade to unsupported instead of
181+
false invalid diagnostics.
182+
- PostgreSQL and BigQuery acceptance remains useful without claiming target
183+
conformance.
184+
- DuckDB compatibility remains useful without pretending to be native
185+
conformance.
186+
- Consumers do not load every node-sql-parser grammar.
187+
- The 16 KiB limit and lack of session wiring intentionally restrict initial
188+
capability.
189+
- Authoritative invalidity requires a separately justified validator and an
190+
owned dialect conformance corpus.
191+
192+
## Non-goals
193+
194+
This decision does not define:
195+
196+
- Stable parser or provider APIs
197+
- Session cache keys or eviction
198+
- Document-level syntax diagnostics
199+
- Semantic relation, scope, column, or type models
200+
- A worker protocol
201+
- Native DuckDB or remote validation providers
202+
- Dremio parsing
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# vNext node-sql-parser Adapter
2+
3+
Status: internal and not wired to sessions
4+
5+
The first concrete syntax adapter exercises the normalized contract from
6+
[ADR 0002](../adr/0002-normalized-syntax-contract.md) under the evidence and
7+
execution policy in
8+
[ADR 0003](../adr/0003-node-sql-parser-adapter.md). It is not exported from
9+
`/vnext` and does not currently power completion, diagnostics, hover,
10+
navigation, or any other editor feature.
11+
12+
## Capability matrix
13+
14+
| Dialect | Backend | Acceptance | Rejection |
15+
| --- | --- | --- | --- |
16+
| PostgreSQL | PostgreSQL-specific build | `parsed/compatibility` with `partial-artifact` | `unsupported/uncovered-construct` |
17+
| BigQuery | BigQuery-specific build | `parsed/compatibility` with `partial-artifact` | `unsupported/uncovered-construct` |
18+
| DuckDB | PostgreSQL-specific build | `parsed/compatibility` with `dialect-compatibility`, `partial-artifact` | `unsupported/compatibility-rejected` |
19+
| Dremio | None | Unavailable when coordinator wiring exists | `unavailable/dialect-not-supported` |
20+
21+
This adapter never returns `direct` or `invalid`. The PostgreSQL and BigQuery
22+
grammars both reject valid target-dialect constructs and accept constructs the
23+
target engines reject. Acceptance therefore records only a partial
24+
compatibility artifact; rejection is not an authoritative syntax diagnostic.
25+
26+
## Loading and packaging
27+
28+
The implementation is tied to an exact `node-sql-parser` 5.4.0 pin and loads
29+
only:
30+
31+
```text
32+
node-sql-parser/build/postgresql.js
33+
node-sql-parser/build/bigquery.js
34+
```
35+
36+
The complete all-dialect entry point is not used. In pure Node, the adapter
37+
dynamically acquires `createRequire`, revalidates the realm, then synchronously
38+
requires the selected build so evaluation and cleanup cannot be interleaved
39+
with another task. Runtime module decoding handles the package's CommonJS shape
40+
rather than trusting its inaccurate named-export declarations.
41+
42+
The adapter invokes `astify` with location collection enabled and query
43+
trimming disabled. Disabling trimming preserves offsets relative to the exact
44+
statement slice. Locations remain partial: PostgreSQL commonly omits root and
45+
identifier locations, while BigQuery provides broader but still incomplete
46+
coverage.
47+
48+
Loading the distributed bundles may write `NodeSQLParser` or `global` on a
49+
global object. The adapter rejects parser loads whenever `window` or `self`
50+
exists, or `global` does not resolve exactly to `globalThis`, before loading a
51+
bundle. This blocks browser windows and Node DOM shims from exposing an
52+
unguarded secondary target. Pure Node loads restore the exact prior descriptors
53+
synchronously after module evaluation, including removing names that were
54+
previously absent. Cleanup failure permanently poisons loading. A
55+
dedicated-worker loader remains future work.
56+
57+
Approximate local Node 24 arm64 measurements for the installed package were:
58+
59+
| Build | Raw size | gzip size | Cold import |
60+
| --- | ---: | ---: | ---: |
61+
| PostgreSQL-specific | 308,145 B | 58,648 B | 19 ms |
62+
| BigQuery-specific | 208,264 B | 41,828 B | 14 ms |
63+
| All dialects | 2,505,457 B | 428,097 B | Not selected |
64+
65+
These figures are investigation evidence, not cross-machine performance
66+
guarantees. Reproducible browser and bundle baselines are required before
67+
session integration.
68+
69+
## Input and output rules
70+
71+
The adapter accepts at most 16 KiB of statement text. Larger statements return
72+
`unsupported/resource-limit` before a backend import or parse.
73+
74+
The input is the exact code-bearing normal statement source:
75+
76+
- Leading and trailing trivia are retained.
77+
- A statement terminator is excluded.
78+
- The source is not trimmed or rewritten.
79+
- The backend receives no editor state, document offset, cursor, catalog, or
80+
host context.
81+
82+
A usable backend result has exactly one object root with a string `type`. The
83+
adapter accepts either that object directly or a one-element array containing
84+
it. Empty or multi-root output is uncovered; structurally malformed output is a
85+
backend contract failure.
86+
87+
Normalized statement kinds are intentionally small:
88+
89+
| Backend type | Normalized kind |
90+
| --- | --- |
91+
| `select`, `union` | `query` |
92+
| `insert`, `replace` | `insert` |
93+
| `update`, `delete`, `create`, `alter`, `drop`, `merge`, `transaction` | Same closed kind |
94+
| Any other string | `other` |
95+
96+
Only the normalized kind and full statement-relative range appear in the
97+
syntax artifact. The raw AST stays in private weakly keyed storage for a future
98+
adapter-owned semantic decoder. Flat `tableList` and `columnList` output is not
99+
used as the semantic model.
100+
101+
## Failure and cancellation behavior
102+
103+
Expected PostgreSQL and BigQuery PEG rejections are uncovered capability, not
104+
failures. DuckDB rejection is compatibility rejection. Unexpected native
105+
exceptions, module-load failures, and malformed module or AST values remain
106+
explicit backend failures; raw exceptions are not retained in results.
107+
108+
Cancellation is checked before and after asynchronous loading and after the
109+
parse. The parse itself is synchronous and cannot be interrupted by an
110+
`AbortSignal` while it blocks JavaScript. A late result can be discarded, but
111+
the CPU work has already occurred. Unsupported execution realms fail closed
112+
without importing a backend.
113+
114+
For that reason, this adapter remains unwired. A worker-versus-main-thread ADR,
115+
with browser latency, memory, hostile-input, timeout, and recovery evidence, is
116+
required before interactive sessions may call it.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
"test:coverage:changed": "node ./scripts/changed-coverage.mjs",
2525
"test:browser": "vitest run --config vitest.browser.config.ts",
2626
"test:integrity": "node ./scripts/check-test-integrity.mjs",
27+
"bench:parser-adapter": "vitest bench --run src/vnext/__tests__/node-sql-parser-adapter.bench.ts",
2728
"bench:statement-index": "vitest bench --run src/vnext/__tests__/statement-index.bench.ts",
2829
"test:package": "node ./scripts/clean.mjs && tsc && node ./scripts/package-smoke.mjs",
2930
"demo": "vite build",
@@ -95,6 +96,6 @@
9596
},
9697
"module": "./dist/index.js",
9798
"dependencies": {
98-
"node-sql-parser": "^5.3.13"
99+
"node-sql-parser": "5.4.0"
99100
}
100101
}

pnpm-lock.yaml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/node-module.d.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
declare module "node:module" {
2+
export function createRequire(
3+
filename: string,
4+
): (specifier: string) => unknown;
5+
}

0 commit comments

Comments
 (0)