Skip to content

fix(deparser): deparse FETCH/MOVE ALL from the WASM AST - #364

Open
pyramation wants to merge 1 commit into
mainfrom
devin/1792899045-fetch-all-wasm
Open

pyramation wants to merge 1 commit into
mainfrom
devin/1792899045-fetch-all-wasm

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

FETCH_ALL is LONG_MAX. In libpg-query's 32-bit WASM build long is 32-bit, so FETCH ALL c parses to howMany: 2147483647 rather than 2^63−1. FetchStmt only checked for the 64-bit value, so it turned FETCH ALL into a literal count:

FETCH ALL FROM c        -> FETCH FORWARD 2147483647 c   (before)
                        -> FETCH FORWARD ALL c          (after)
MOVE BACKWARD ALL IN c  -> MOVE BACKWARD 2147483647 c   (before)
                        -> MOVE BACKWARD ALL c          (after)

The AST round-trip didn't catch this, because the reparsed SQL yields 2147483647 again. isAll now accepts either sentinel. That matches plpgsql-deparser, which already treats how_many === 2147483647 as ALL.

Trade-off: a literal FETCH 2147483647 now deparses as FETCH ALL. The WASM AST can't tell the two apart anyway.

Tests

  • Added two statements to the kitchen-sink fixture pretty/misc.sql (misc-18, misc-19), ran npm run kitchen-sink, and added them to __tests__/pretty/misc-pretty.test.ts. The new snapshots are FETCH FORWARD ALL c and MOVE BACKWARD ALL c.
  • Without the deparser.ts change: Tests: 4 failed, 34 passed (Received: "FETCH FORWARD 2147483647 c", "MOVE BACKWARD 2147483647 c").
  • With it: packages/deparser, npx jest: 312 suites, 752 tests passing.

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
Contributor

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 issues found — approved ✅.


This PR fixes FetchStmt deparsing so that FETCH ALL / MOVE ALL statements parsed through the 32-bit WASM build of libpg-query still deparse back to the ALL keyword instead of a literal count. Two kitchen-sink fixtures (FETCH ALL FROM c, MOVE BACKWARD ALL IN c) with tests and snapshots cover both fetch and move paths.

Files Change
packages/deparser/src/deparser.ts Adds the 2^31-1 sentinel check for node.howMany alongside the existing 2^63-1 LONG_MAX value when deciding to emit ALL.
__fixtures__/kitchen-sink/pretty/misc.sql Adds FETCH ALL FROM c and MOVE BACKWARD ALL IN c fixtures.
__fixtures__/generated/generated.json Regenerated fixture metadata for the new statements.
packages/deparser/__tests__/pretty/misc-pretty.test.ts and snapshot Registers the new cases and pins expected pretty output.
packages/deparser/__tests__/kitchen-sink/pretty-misc.test.ts Registers the new fixtures in the kitchen-sink suite.

Reviewed commit: 0ead3fc

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Folded into #371

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