fix(plpgsql-deparser): fix PERFORM, INTO, and recfield bugs#266
Merged
pyramation merged 3 commits intomainfrom Jan 6, 2026
Merged
fix(plpgsql-deparser): fix PERFORM, INTO, and recfield bugs#266pyramation merged 3 commits intomainfrom
pyramation merged 3 commits intomainfrom
Conversation
- Fix PERFORM SELECT: Strip leading SELECT keyword since PERFORM replaces it - Fix INTO clause insertion: Use depth-aware scanner to avoid inserting INTO inside subqueries - Fix recfield qualification: Use recparentno to construct full qualified reference (e.g., new.is_active) Add comprehensive snapshot tests for all three fixes.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
The parser strips 'INTO <target>' from the query but leaves whitespace behind. This fix normalizes the leading whitespace after the insertion point to avoid large gaps like 'SELECT x INTO y FROM z'.
Upgrade deparser-fixes tests to use round-trip testing infrastructure: - parse -> deparse -> reparse -> compare cleaned ASTs - Add normalizeQueryWhitespace to cleanPlpgsqlTree to handle indentation differences in embedded SQL queries - All 17 test cases now verify AST equality after round-trip
4 tasks
devin-ai-integration bot
pushed a commit
that referenced
this pull request
Jan 6, 2026
… PR #229 Add 14 new PL/pgSQL fixtures that exercise the deparser fixes: - PERFORM statement (parser stores as SELECT, deparser strips SELECT) - INTO clause with record field target (recfield qualification) - INTO clause with subquery (depth-aware scanner) - SETOF function with RETURN QUERY and bare RETURN - SETOF function with RETURN NEXT - Void function with bare RETURN - Scalar function with RETURN NULL - OUT parameter function with bare RETURN - RETURNS TABLE function with RETURN QUERY - Trigger functions with complex logic - Procedure (implicit void return) These fixtures are inspired by real-world functions from constructive-db that required the deparser fixes in PR #266. Regenerated generated.json: 190 valid statements (up from 176)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes three bugs in the plpgsql-deparser that were causing invalid SQL output:
PERFORM SELECT fix: The PL/pgSQL parser stores PERFORM statements internally as "SELECT ...", so the deparser was outputting invalid
PERFORM SELECT ...syntax. Now strips the leading SELECT keyword.INTO clause depth-aware scanner: The old regex-based approach for inserting INTO clauses would incorrectly insert INTO inside subqueries. Replaced with a depth-aware scanner (~150 lines) that tracks parentheses, quotes, comments, and dollar quotes to find the correct insertion point only at depth 0.
Record field qualification: The deparser was returning just
fieldnameinstead of the full qualified reference likenew.is_active. Now usesrecparentnoto look up the parent record and construct the full reference.Updates since last revision
Whitespace normalization fix: The parser strips
INTO <target>from the stored query but leaves the original whitespace behind. This was causing large gaps in output likeSELECT x INTO y FROM z. Now normalizes leading whitespace after INTO insertion to a single space.Round-trip AST testing: Upgraded all 17 test cases to use round-trip testing (parse → deparse → reparse → compare cleaned ASTs). Added
normalizeQueryWhitespacetocleanPlpgsqlTreeto handle indentation differences in embedded SQL queries while preserving content inside string literals and dollar-quoted strings.Review & Testing Checklist for Human
findIntoInsertionPointfunction (lines 1090-1243) is complex state machine logic. Test with: nested subqueries, CTEs, UNION, dollar-quoted strings containing "FROM", comments containing keywords, escaped quotesnormalizeQueryWhitespacefunction: This function (~100 lines in test-utils/index.ts) normalizes whitespace for AST comparison. Verify it doesn't mask real semantic differences by being too aggressiveRecommended test plan:
Notes
This PR upstreams patches 1-3 from constructive-db PR #229. Patch 4 (RETURN statement handling) is being handled separately as it requires design discussion about passing return-type context.
Link to Devin run: https://app.devin.ai/sessions/8e1c971e9b194cd9a7dda034c89bd74b
Requested by: Dan Lynch (@pyramation)