Repository navigation
chore: combined merge of #364, #367, #365, #366, #368, #369, #370 - #371
pyramation wants to merge 17 commits into
Conversation
… ones; clamp scanner paren depth
…g, COPY values, tablespaces and WITH TIES
… in normalizeTree
…nd empty CASE ELSE intact
…known-failures ratchet
… SHOW quoting, COPY values, tablespaces and WITH TIES
…TS and JSON_TABLE
…validate operator names
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
🧹 Nitpicks (3) — 🟢 3 low
The change introduces a PostgreSQL regression-statement corpus generator and round-trip ratchet tests for both deparser packages, then fixes a series of deparse-fidelity bugs surfaced by that corpus. All new fixtures pair 1:1 with generated data and test files, and the codegen metadata changes align with the deparser updates.
Reviewed commit: 85078d9 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds a regression-corpus ratchet for the SQL and PL/pgSQL deparsers plus a batch of deparser fidelity fixes (FETCH FIRST WITH TIES, operator-name validation, SHOW/ALTER STATISTICS/tablespace/JSON_TABLE handling, PL/pgSQL ELSE and indent scanning).
Key findings
- 🟡 Dollar-tag scan copies whole remaining text per
$— plpgsql-deparser.ts:2449
| } | ||
| if (ch === '$') { | ||
| const prev = start > 0 ? text[start - 1] : ''; | ||
| const tag = /^\$([A-Za-z_\u0080-\uffff][A-Za-z0-9_\u0080-\uffff]*)?\$/.exec(text.slice(start)); |
There was a problem hiding this comment.
🟡 performance · medium
Dollar-tag scan copies whole remaining text per $
The new indent loop calls skipQuoted for every $ character, and the $ branch does /^\$...\$/.exec(text.slice(start)) (packages/plpgsql-deparser/src/plpgsql-deparser.ts:2449) — a full copy of the remaining string per $. PL/pgSQL bodies contain many $ tokens ($1/$2 parameters, dollar-quoted strings), so deparsing a large body does O(n²) memory traffic where the previous single-pass regex replace was linear. The loop itself is fine; the slice is the quadratic part.
📋 Prompt for AI Agents
In packages/plpgsql-deparser/src/plpgsql-deparser.ts at line 2449, replace the text.slice(start) argument with a sticky-anchored match: create a module-level regex /^\$([A-Za-z_\u0080-\uffff][A-Za-z0-9_\u0080-\uffff]*)?\$/y, set regex.lastIndex = start before each exec against text, and read the match — this avoids copying the entire remainder of the string for every $ in the body.
Summary
One merge PR that folds seven open PRs in, in merge order, using merge commits (no rebase). If this merges, the individual PRs can be closed.
FETCH ALL/MOVE ALLfrom the WASM AST (howMany= 2147483647) deparsed as a numeric count → deparses asALLnormalizeTreeskipped nodes under aconstructorkey (JSON_ARRAYAGG/JSON_OBJECTAGG), which caused false round-trip diffs → those nodes are normalized"char"intocharand dropped an emptyCASE ... ELSE→ all three kept intactAND CHAIN, trigger names inALTER TRIGGER ... RENAME,XMLSERIALIZE ... INDENT,SHOWquoting, COPY option values, tablespaces/index options andFETCH ... WITH TIESwere silently dropped or changed → preservedJSON_QUERY/JSON_VALUE/JSON_EXISTS/JSON_TABLEthrew "does not handle node type" → deparsedALTER STATISTICS ... RENAMEthrew, and operator names were emitted unquoted or unvalidated, so the SQL could reparse as a different operator → supported, quoted and validatedConflicts were resolved by regenerating, not by hand-editing. The only conflict was
__fixtures__/generated/generated.json(from #370), fixed withnpm run fixtures.npm run kitchen-sink(deparser) andnpm run fixtures(plpgsql-deparser) were then re-run on the merged tree with no diff. The build re-inferredpackages/pgsql-types/src/field-metadata.jsonfrom the new fixtures:stxstattargetis now nullable andParamRefwas added as a tag.Ratchet (#366 known failures), regenerated with
UPDATE_KNOWN_FAILURES=1The update only removed entries; no new failures were added.
Checks
pnpm run build: OK.pnpm run lint: 0 errors.pnpm -r --no-bail run test: deparser 1180, transform 255, plpgsql-deparser 116, transform-ast 1443, plpgsql-parser 345 and all other packages pass. Three things fail locally, all outside the CI matrix:@pgsql/cliandpgsql-typeshave no tests, and theplpgsql-parseroundtrip snapshot fails onmainas well.Link to Devin session: https://app.devin.ai/sessions/cb07c6509db64478af655f52611468f4
Open in Devin Desktop: https://app.devin.ai/desktop/session/cb07c6509db64478af655f52611468f4?variant=devin
Requested by: @pyramation