Skip to content

fix(plpgsql-deparser): support nested DECLARE blocks inside FOR loops #271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pyramation merged 1 commit into from
Jan 7, 2026
Merged

fix(plpgsql-deparser): support nested DECLARE blocks inside FOR loops #271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions packages/plpgsql-deparser/__tests__/plpgsql-deparser.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ describe('PLpgSQLDeparser', () => {
});

describe('round-trip tests using generated.json', () => {
// Known failing fixtures due to pre-existing deparser issues:
// TODO: Fix these underlying issues and remove from allowlist
// Remaining known failing fixtures:
// - plpgsql_varprops-13.sql: nested DECLARE inside FOR loop - variables declared inside
// the loop body are hoisted to the top-level DECLARE section, changing semantics
// (variables should be reinitialized on each loop iteration)
const KNOWN_FAILING_FIXTURES = new Set([
'plpgsql_varprops-13.sql',
// All 190 fixtures now pass round-trip testing!
// Previously failing fixtures that have been fixed:
// - Schema qualification (pg_catalog prefix preserved)
// - Exception block handling
// - Cursor FOR loops
// - Labeled blocks with EXIT statements
// - Nested DECLARE inside FOR loops (lineno-based scope tracking)
const KNOWN_FAILING_FIXTURES = new Set<string>([
// No known failures - all fixtures pass!
]);

it('should round-trip ALL generated fixtures (excluding known failures)', async () => {
Expand Down
258 changes: 248 additions & 10 deletions packages/plpgsql-deparser/src/plpgsql-deparser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ export interface PLpgSQLDeparserContext {
options: PLpgSQLDeparserOptions;
datums?: PLpgSQLDatum[];
returnInfo?: ReturnInfo;
/** Set of linenos for loop-introduced variables (to exclude from DECLARE) */
loopVarLinenos?: Set<number>;
/** Map of block lineno to the set of datum indices that belong to that block */
blockDatumMap?: Map<number, Set<number>>;
}

/**
Expand Down Expand Up @@ -154,11 +158,30 @@ export class PLpgSQLDeparser {
* @param returnInfo - Optional return type info for correct RETURN statement handling
*/
deparseFunction(func: PLpgSQL_function, returnInfo?: ReturnInfo): string {
// Collect loop-introduced variables before generating DECLARE section
const loopVarLinenos = new Set<number>();
if (func.action) {
this.collectLoopVariables(func.action, loopVarLinenos);
}

// Build the block-to-datum mapping for nested DECLARE sections
const blockDatumMap = this.buildBlockDatumMap(func.datums, func.action, loopVarLinenos);

// Collect all datum indices that belong to nested blocks (to exclude from top-level)
const nestedDatumIndices = new Set<number>();
for (const indices of blockDatumMap.values()) {
for (const idx of indices) {
nestedDatumIndices.add(idx);
}
}

const context: PLpgSQLDeparserContext = {
indentLevel: 0,
options: this.options,
datums: func.datums,
returnInfo,
loopVarLinenos,
blockDatumMap,
};

const parts: string[] = [];
Expand All @@ -175,14 +198,14 @@ export class PLpgSQLDeparser {
parts.push(`<<${blockLabel}>>`);
}

// Collect loop-introduced variables before generating DECLARE section
const loopVarLinenos = new Set<number>();
if (func.action) {
this.collectLoopVariables(func.action, loopVarLinenos);
}

// Deparse DECLARE section (local variables, excluding loop variables)
const declareSection = this.deparseDeclareSection(func.datums, context, loopVarLinenos);
// Deparse DECLARE section (local variables, excluding loop variables and nested block variables)
const declareSection = this.deparseDeclareSection(
func.datums,
context,
loopVarLinenos,
undefined, // includedIndices - not used for top-level
nestedDatumIndices // excludedIndices - exclude datums that belong to nested blocks
);
if (declareSection) {
parts.push(declareSection);
}
Expand Down Expand Up @@ -339,20 +362,221 @@ export class PLpgSQLDeparser {
}
}

/**
* Build a mapping of block linenos to the datum indices that belong to each block.
* This is used to emit DECLARE sections at the correct nesting level.
*
* The algorithm:
* 1. Get the top-level block's lineno (the BEGIN line of the function body)
* 2. Collect all nested PLpgSQL_stmt_block linenos from the AST
* 3. For each datum with a lineno GREATER than the top-level block's lineno:
* - Assign it to the nested block whose lineno is the smallest value greater than the datum's lineno
* 4. Datums with lineno <= top-level block lineno belong to the top-level DECLARE (not added to map)
*/
private buildBlockDatumMap(
datums: PLpgSQLDatum[] | undefined,
action: PLpgSQLStmtNode | undefined,
loopVarLinenos: Set<number>
): Map<number, Set<number>> {
const blockDatumMap = new Map<number, Set<number>>();

if (!datums || !action) {
return blockDatumMap;
}

// Get the top-level block's lineno
let topLevelBlockLineno: number | undefined;
if ('PLpgSQL_stmt_block' in action) {
topLevelBlockLineno = action.PLpgSQL_stmt_block.lineno;
}

// Collect all nested block linenos (excluding the top-level block)
const nestedBlockLinenos: number[] = [];
this.collectNestedBlockLinenos(action, nestedBlockLinenos, true);
nestedBlockLinenos.sort((a, b) => a - b);

// For each datum, find which block it belongs to
datums.forEach((datum, index) => {
let lineno: number | undefined;

if ('PLpgSQL_var' in datum) {
lineno = datum.PLpgSQL_var.lineno;
} else if ('PLpgSQL_rec' in datum) {
lineno = datum.PLpgSQL_rec.lineno;
} else if ('PLpgSQL_row' in datum) {
lineno = datum.PLpgSQL_row.lineno;
}

// Skip datums without lineno or loop variables
if (lineno === undefined || loopVarLinenos.has(lineno)) {
return;
}

// Only consider datums declared AFTER the top-level BEGIN for nested blocks
// Datums declared before the top-level BEGIN belong to the top-level DECLARE
// If topLevelBlockLineno is undefined, we can't determine scope, so keep all at top-level
if (topLevelBlockLineno === undefined || lineno <= topLevelBlockLineno) {
return; // This datum belongs to top-level DECLARE
}

// Find the block this datum belongs to (the next BEGIN after the datum's lineno)
for (const blockLineno of nestedBlockLinenos) {
if (blockLineno > lineno) {
// This datum belongs to this block
if (!blockDatumMap.has(blockLineno)) {
blockDatumMap.set(blockLineno, new Set());
}
blockDatumMap.get(blockLineno)!.add(index);
return;
}
}
// If no nested block found, datum belongs to top-level (not added to map)
});

return blockDatumMap;
}

/**
* Collect linenos of all nested PLpgSQL_stmt_block nodes in the AST.
* @param isTopLevel - If true, skip the current block (it's the top-level block)
*/
private collectNestedBlockLinenos(
stmt: PLpgSQLStmtNode,
linenos: number[],
isTopLevel: boolean = false
): void {
if ('PLpgSQL_stmt_block' in stmt) {
const block = stmt.PLpgSQL_stmt_block;
// Only add nested blocks, not the top-level block
if (!isTopLevel && block.lineno !== undefined) {
linenos.push(block.lineno);
}
if (block.body) {
for (const s of block.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_fori' in stmt) {
const fori = stmt.PLpgSQL_stmt_fori;
if (fori.body) {
for (const s of fori.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_fors' in stmt) {
const fors = stmt.PLpgSQL_stmt_fors;
if (fors.body) {
for (const s of fors.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_forc' in stmt) {
const forc = stmt.PLpgSQL_stmt_forc;
if (forc.body) {
for (const s of forc.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_foreach_a' in stmt) {
const foreach = stmt.PLpgSQL_stmt_foreach_a;
if (foreach.body) {
for (const s of foreach.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_loop' in stmt) {
const loop = stmt.PLpgSQL_stmt_loop;
if (loop.body) {
for (const s of loop.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_while' in stmt) {
const whileStmt = stmt.PLpgSQL_stmt_while;
if (whileStmt.body) {
for (const s of whileStmt.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_if' in stmt) {
const ifStmt = stmt.PLpgSQL_stmt_if;
if (ifStmt.then_body) {
for (const s of ifStmt.then_body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
if (ifStmt.elsif_list) {
for (const elsif of ifStmt.elsif_list) {
if ('PLpgSQL_if_elsif' in elsif && elsif.PLpgSQL_if_elsif.stmts) {
for (const s of elsif.PLpgSQL_if_elsif.stmts) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
}
}
if (ifStmt.else_body) {
for (const s of ifStmt.else_body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_case' in stmt) {
const caseStmt = stmt.PLpgSQL_stmt_case;
if (caseStmt.case_when_list) {
for (const when of caseStmt.case_when_list) {
if ('PLpgSQL_case_when' in when && when.PLpgSQL_case_when.stmts) {
for (const s of when.PLpgSQL_case_when.stmts) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
}
}
if (caseStmt.have_else && caseStmt.else_stmts) {
for (const s of caseStmt.else_stmts) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
} else if ('PLpgSQL_stmt_dynfors' in stmt) {
const dynfors = stmt.PLpgSQL_stmt_dynfors;
if (dynfors.body) {
for (const s of dynfors.body) {
this.collectNestedBlockLinenos(s, linenos, false);
}
}
}
}

/**
* Deparse the DECLARE section
* @param datums - All datums from the function
* @param context - Deparser context
* @param loopVarLinenos - Set of linenos for loop-introduced variables to exclude
* @param includedIndices - Optional set of datum indices to include (for nested blocks).
* If provided, only datums at these indices are included.
* @param excludedIndices - Optional set of datum indices to exclude (for top-level).
* If provided, datums at these indices are excluded.
*/
private deparseDeclareSection(
datums: PLpgSQLDatum[] | undefined,
context: PLpgSQLDeparserContext,
loopVarLinenos: Set<number> = new Set()
loopVarLinenos: Set<number> = new Set(),
includedIndices?: Set<number>,
excludedIndices?: Set<number>
): string {
if (!datums || datums.length === 0) {
return '';
}

// Filter out internal variables (like 'found', parameters, etc.) and loop variables
const localVars = datums.filter(datum => {
const localVars = datums.filter((datum, index) => {
// If includedIndices is provided, only include datums at those indices
if (includedIndices !== undefined && !includedIndices.has(index)) {
return false;
}
// If excludedIndices is provided, exclude datums at those indices
if (excludedIndices !== undefined && excludedIndices.has(index)) {
return false;
}

if ('PLpgSQL_var' in datum) {
const v = datum.PLpgSQL_var;
// Skip internal variables:
Expand Down Expand Up @@ -580,6 +804,20 @@ export class PLpgSQLDeparser {
parts.push(`<<${block.label}>>`);
}

// Check if this block has any datums assigned to it (nested DECLARE)
if (block.lineno !== undefined && context.blockDatumMap?.has(block.lineno)) {
const includedIndices = context.blockDatumMap.get(block.lineno)!;
const declareSection = this.deparseDeclareSection(
context.datums,
context,
context.loopVarLinenos || new Set(),
includedIndices
);
if (declareSection) {
parts.push(declareSection);
}
}

parts.push(kw('BEGIN'));

// Body statements
Expand Down