feat(plpgsql-deparser): add context-based RETURN statement handling#267
Merged
pyramation merged 4 commits intomainfrom Jan 6, 2026
Merged
feat(plpgsql-deparser): add context-based RETURN statement handling#267pyramation merged 4 commits intomainfrom
pyramation merged 4 commits intomainfrom
Conversation
Add ReturnInfo type and context to PLpgSQLDeparserContext for correct RETURN statement syntax based on function return type: - void/setof/trigger/out_params: bare RETURN is valid - scalar: RETURN NULL is required for empty returns Add getReturnInfo helper to plpgsql-parser to extract return type info from CreateFunctionStmt AST node. When returnInfo is provided in context, the deparser uses it to determine the correct RETURN syntax. When not provided, it falls back to bare RETURN for backward compatibility. Includes tests for all return type scenarios (void, setof, trigger, scalar, OUT params, RETURNS TABLE).
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:
|
…erated fixtures Add test that runs round-trip assertions on all 176 fixtures from plpgsql-generated/generated.json. This ensures the deparser produces semantically equivalent output for all canonical PL/pgSQL test cases. 16 fixtures are in a known-failing allowlist due to pre-existing issues: - Schema qualification loss (pg_catalog.pg_class%rowtype[] -> pg_class%rowtype[]) - Tagged dollar quote reconstruction ($tag$...$tag$ not supported) - Exception block handling issues The test will fail if any NEW fixtures start failing (regression detection) while allowing the known issues to be tracked and fixed incrementally. Also improved cleanPlpgsqlTree to normalize 'undefined vs missing' properties for more accurate AST comparison.
… 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)
Both packages have tests that should be run in CI: - plpgsql-deparser: comprehensive round-trip tests for 190 fixtures - plpgsql-parser: return-info extraction tests
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.
feat(plpgsql-deparser): add context-based RETURN statement handling
Summary
This PR adds context-based RETURN statement handling to the PL/pgSQL deparser. PostgreSQL requires different RETURN syntax based on function return type:
RETURNis validRETURN NULLis required for empty returnsPreviously, the deparser always output bare
RETURNfor empty return statements. Now, callers can passReturnInfocontext to get correct output based on the function's return type.Key changes:
ReturnInfotype with kinds:void,setof,trigger,scalar,out_paramsdeparseReturn()to use context when available, defaulting to bareRETURNfor backward compatibilitygetReturnInfo()helper inplpgsql-parserto extract return type fromCreateFunctionStmtASTThe snapshot changes reflect previous PERFORM/INTO fixes from PR #266 that were merged to main.
Updates since last revision
Latest: Added plpgsql-parser and plpgsql-deparser to CI workflow
Previous: Added 14 regression fixtures from constructive-db PR #229
__fixtures__/plpgsql/plpgsql_deparser_fixes.sqlwith real-world-inspired test casesgenerated.json: now 190 valid statements (up from 176)Previous: Comprehensive round-trip testing for ALL generated fixtures
plpgsql-generated/generated.jsonpg_catalog.pg_class%rowtype[]→pg_class%rowtype[])$tag$...$tag$not supported)Review & Testing Checklist for Human
getReturnInfo()logic inpackages/plpgsql-parser/src/return-info.ts- ensure the AST parsing correctly identifies all return type scenarios (especially edge cases likeRETURNS TABLEvsOUTparams)mainbranch too, not just this PRplpgsql-parserandplpgsql-deparserjobs appear in the workflow run__fixtures__/plpgsql/plpgsql_deparser_fixes.sql- verify they cover the patterns that required fixes in constructive-dbSuggested test plan:
pnpm testinpackages/plpgsql-deparserandpackages/plpgsql-parserRETURN NULLwhen context is providedRETURNNotes
This is patch 4 from constructive-db PR #229, implementing a context-passing approach instead of heuristics for RETURN handling.
Link to Devin run: https://app.devin.ai/sessions/8e1c971e9b194cd9a7dda034c89bd74b
Requested by: Dan Lynch (@pyramation)