Skip to content

fix(v18): use pg_query_scan_tokens for scan - #171

Open
pyramation wants to merge 2 commits into
mainfrom
fix/pg18-scan-tokens
Open

pyramation wants to merge 2 commits into
mainfrom
fix/pg18-scan-tokens

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

Merge after constructive-io/libpg_query chore/sync-18-latest → 18-constructive. This PR's CI clones 18-constructive, and it only builds once that merge lands.

Upstream libpg_query switched from protobuf-c to upb and removed protobuf/pg_query.pb-c.h. The full-API wasm_wrapper.c included that header in order to unpack pg_query_scan() output, so the v18 WASM build fails to compile against the synced fork.

The fix uses upstream's new protobuf-free API:

- PgQueryScanResult r = pg_query_scan(input);  /* + pg_query__scan_result__unpack */
+ PgQueryScanTokensResult r = pg_query_scan_tokens(input);  /* tokens[i].{start,end,token,keyword_kind} */

scan() JSON output stays the same: same fields, same simplified tokenName mapping. version now comes from PG_VERSION_NUM, which is the same value protobuf used to carry. Tests and README now expect 180006 (PG 18.6).

Verified locally: v18 pnpm build + pnpm test pass 92/92 when built from the synced fork branch.

Link to Devin session: https://app.devin.ai/sessions/bb737daf640f408b8e33e9f72dc75587
Open in Devin Desktop: https://app.devin.ai/desktop/session/bb737daf640f408b8e33e9f72dc75587?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 3, 2026 •

Copy link
Copy Markdown

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

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Vendored libpg_query.md still documents removed scan API (libpg_query.md) — The PR migrates the wrapper to pg_query_scan_tokens/PgQueryScanTokensResult (versions/18/src/wasm_wrapper.c:378), but the vendored API reference versions/18/libpg_query.md still documents the removed protobuf-based pg_query_scan with an unpack workflow (lines 52-77 and 247).

The change moves both C wrappers (templates/full and versions/18) to the new pg_query_scan_tokens API: protobuf unpacking is removed, PgQueryScanTokensResult fields are read directly as a struct array, cleanup switches to pg_query_free_scan_tokens_result, and the reported version now comes from the PG_VERSION_NUM build constant. The emitted JSON shape ({"version":N,"tokens":[...]}) is unchanged, so the TypeScript scan/scanSync consumers remain compatible. The test suite in versions/18/test/pg18.test.js was updated for the new parser version, and versions/18/README.md documents the version bump. One low-severity doc divergence remains: the vendored API reference still describes the removed scan API.

Files Change
templates/full/wasm_wrapper.c, versions/18/src/wasm_wrapper.c Migrated to pg_query_scan_tokens/pg_query_free_scan_tokens_result, dropping protobuf includes and unpack code; token loops now iterate scan_result->tokens as structs and version is emitted from PG_VERSION_NUM.
versions/18/README.md Version bump documentation to the new parser build.
versions/18/test/pg18.test.js Test expectations updated for the pg18 parser and scan token output.
versions/18/libpg_query.md (untouched) Still documents the removed pg_query_scan protobuf workflow; flagged for follow-up.

Reviewed commit: cfff958

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