Skip to content

ci: check WASM parse output against native libpg_query on its test SQL - #187

Open
pyramation wants to merge 2 commits into
mainfrom
devin/1792899704-native-parity
Open

pyramation wants to merge 2 commits into
mainfrom
devin/1792899704-native-parity

Conversation

@pyramation

@pyramation pyramation commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds a Native parity 🔍 step to the existing build-wasm matrix job. No new job. Right after pnpm run build, scripts/native-parity.js versions/<n> does the following:

  1. Exports the libpg_query checkout that the WASM build already cloned (.cache/*/libpg_query/<x-publish.libpgQueryCommit>) to a temp dir with git archive, then runs a native make build. No network is needed.
  2. Compiles scripts/native-parity/driver.c, which wraps pg_query_split_with_scanner and pg_query_parse.
  3. Splits libpg_query's test SQL (test/sql/postgres_regress, plpgsql_regress, deparse*, test/plpgsql_samples.sql) into statements.
  4. Checks that the freshly built wasm/index.cjs parseSync gives byte-identical JSON or the same error (message + cursor) for every statement. It exits 1 and prints the first 10 differences otherwise.

Native and WASM have one known difference, which the script maps instead of reporting. FETCH_ALL is LONG_MAX, so howMany/how_many is 2^63−1 natively and 2147483647 on wasm32.

On glibc ≥ 2.38 (the ubuntu-24.04 runners), PG ≤ 14's vendored snprintf.c won't compile natively (conflicting types for 'strchrnul'). The native build therefore gets CPPFLAGS=-DHAVE_STRCHRNUL= on Linux. This only affects the parity build, not the WASM.

This catches WASM build regressions (emsdk bump, wrapper or Makefile changes, fork bumps) against the C library itself, using the full PostgreSQL regression suite as input.

Evidence: this PR's CI run

version statements files differences step time
v13 @ 1097b2c 34,670 204 0 26 s
v14 @ 397cbb9 37,532 223 0 19 s
v15 @ ae10504 40,450 225 0 40 s
v16 @ fce106a 42,445 229 0 19 s
v17 @ 1ade880 46,032 402 0 20 s
v18 @ b0f5f33 49,655 411 0 13 s

Negative check (local): changing the FETCH_ALL mapping makes it report 58 differences (combocid.sql#26, hash_index.sql#54, …) and exit 1.

Parse only for now. On main, v18 fingerprint/normalize/scan still return some errors as results (fixed in #183), so they would differ from native until that lands.

Link to Devin session: https://app.devin.ai/sessions/91c28df261ac4b318f39d1c7242d008c
Open in Devin Desktop: https://app.devin.ai/desktop/session/91c28df261ac4b318f39d1c7242d008c?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review complete. No blocking issues — approved ✅; 1 nitpick below.

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Tempdir cleanup skipped on every failure path (native-parity.js:69) — The temporary workdir work is removed only at the very end of the happy path (scripts/native-parity.js:69); none of the execFileSync calls (make, cc, driver, git archive) are wrapped in try/finally, so any failure leaks the full libpg_query checkout, build objects, and statements.bin in os.tmpdir().

This PR introduces a CI parity harness that builds libpg_query natively, drives it with a small C program over the JSON API, and compares its results statement-by-statement against the WASM bindings to catch divergence between the two builds.

Files Change
.github/workflows/ci.yml Adds a parity job/step that invokes the new script during CI runs.
scripts/native-parity.js New orchestration script: builds natively, runs the driver, loads the WASM module, and diffs parse/split results.
scripts/native-parity/driver.c New C driver that feeds SQL to the native library over stdin and emits JSON results for comparison.

Reviewed commit: 4761bd1

@devin-ai-integration

Copy link
Copy Markdown

Folded into #190

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.

1 participant