[pyramation]

Summary

check_stack_depth() is compiled into libpg_query but is dead code: stack_base_ptr is only set by set_stack_base(), which PostgreSQL calls from main() and the library never calls. So deep recursion in the tree walkers runs until the C stack overflows — in the WebAssembly build that is a RuntimeError: memory access out of bounds (the Aug-2025 crash in libpg-query-node), and the instance is left corrupt.

This PR makes the guard usable and turns those traps into PostgreSQL's normal stack depth limit exceeded error:

• pg_query_set_stack_limit(size_t bytes) (in src_backend_utils_misc_stack_depth.c): sets stack_base_ptr to the caller's frame and max_stack_depth_bytes/max_stack_depth to bytes. An embedder calls it at every entry with however much stack it actually has left (libpg-query-node: emscripten_stack_get_free() - 512 KiB).
• check_stack_depth() at the top of the recursive entry points that had none: _outNode (JSON and protobuf), _fingerprintNode, dump_stmt / dump_expr (PL/pgSQL JSON), deparseExpr. With stack_base_ptr == NULL (embedders that never call the setter) it stays a no-op, so nothing changes for them.
• pg_query_parse_opts: pg_query_nodes_to_json ran outside the PG_TRY in pg_query_raw_parse, so an ERROR raised while emitting JSON had no handler. It now runs under its own PG_TRY/PG_CATCH and is returned as result.error, like a parse error.

Verified in libpg-query-node: with a Node stack large enough to exhaust the 32 MiB WASM stack (ulimit -s unlimited; node --stack-size=100000), SELECT 1+1+… with 250k terms traps on the current build and returns stack depth limit exceeded with this patch; the next call works. make build, make test pass.

Also: free the PL/pgSQL alias list (pre-existing valgrind failure)

The valgrind CI jobs were already red on unpatched 18-constructive (verified on a clean worktree: test/parse_plpgsql exits 1 with 192 bytes … still reachable from pg_query_plpgsql_record_alias): pg_query_plpgsql_reset_aliases() freed the names but never the array, and nothing called it after the last compile. Earlier fork PRs never ran this workflow, so it went unnoticed. Second commit: reset frees the array too and pg_query_parse_plpgsql() resets at the end, so nothing outlives the call. make test VALGRIND=1 is now clean (in use at exit: 0 bytes).

Companion: libpg-query-node PR pganalyze#180 (wrapper calls the setter per export, bumps the pin). Same patch for 17-constructive in #11.

Link to Devin session: https://app.devin.ai/sessions/2a9e06519f424ebab14772c74c05dd83
Open in Devin Desktop: https://app.devin.ai/desktop/session/2a9e06519f424ebab14772c74c05dd83?variant=devin
Requested by: @pyramation

[tenki-reviewer]

Review complete. 🟠 2 high · 🟡 3 medium

💬 Inline comments (2)

• 🟡 New check_stack_depth guards are inert; setter undeclared — src_backend_utils_misc_stack_depth.c:151
• 🟡 Serialize-on-parse-error path left outside PG_TRY — pg_query_parse.c:165

📍 Findings outside the diff (3) — 🟠 2 high · 🟡 1 medium — defects on lines GitHub can't attach comments to

🟠 High — Fingerprint entry points cannot catch new stack-depth error · pg_query_fingerprint.c:343–350 · unchanged line

// src/pg_query_fingerprint.c
343		if (parsetree_and_error.tree != NULL || result.error == NULL) {
344			FingerprintContext ctx;
345			XXH64_canonical_t chash;
346	
347			_fingerprintInitContext(&ctx, NULL, printTokens);
348	
349			if (parsetree_and_error.tree != NULL) {
350				_fingerprintNode(&ctx, parsetree_and_error.tree, NULL, NULL, 0);

check_stack_depth() was added to _fingerprintNode (src/pg_query_fingerprint.c:267), so fingerprinting can now raise an ereport(ERROR) whenever an embedder has installed a stack limit via pg_query_set_stack_limit. But both callers of _fingerprintNode run it with no PG_TRY/PG_CATCH: pg_query_fingerprint_node (src/pg_query_fingerprint.c:320) and pg_query_fingerprint_with_opts (src/pg_query_fingerprint.c:350). With PG_exception_stack NULL, elog.c promotes the ERROR to FATAL and calls proc_exit(1), killing the embedding process instead of returning result.error. Previously fingerprint recursion was hard-capped at depth 100 and could never throw, so this is a new uncaught-error path; the PR added a PG_TRY for the identical error class in pg_query_parse_opts but not here.

---

🟠 High — Protobuf parse path lacks handler for stack-depth throw · pg_query_parse.c:195 · unchanged line

// src/pg_query_parse.c
195		result.parse_tree = pg_query_nodes_to_protobuf(parsetree_and_error.tree);

The PR adds check_stack_depth() to the protobuf serializer _outNode (src/pg_query_outfuncs_protobuf.c:250), so pg_query_nodes_to_protobuf can now raise ereport(ERROR) for deep parse trees — yet pg_query_parse_protobuf_opts calls it unguarded at src/pg_query_parse.c:195. With no PG_TRY on this path, the throw longjmps out of the library: pg_query_exit_memory_context at line 197 is skipped, the per-call memory context and error state leak, and the FATAL-promoted error terminates the embedding process. The sibling JSON path in the same file received a PG_TRY/PG_CATCH for exactly this error, leaving the protobuf path inconsistent.

---

🟡 Medium — Guard statement-level deparse recursion, not just deparseExpr · postgres_deparse.c:11841–11855 · unchanged line

// src/postgres_deparse.c
11841	static void deparseStmt(DeparseState *state, Node *node)
11842	{
11843		DeparseStateNestingLevel *parent_level = NULL;
11844	
11845		// For statements that can be nested, push/pop is handled directly in the
11846		// respective deparse...Stmt methods
11847		bool skip_push_pop = IsA(node, SelectStmt) ||
11848			IsA(node, InsertStmt) ||
11849			IsA(node, UpdateStmt) ||
11850			IsA(node, DeleteStmt) ||
11851			IsA(node, MergeStmt);
11852	

The PR adds check_stack_depth() only in deparseExpr (src/postgres_deparse.c:788), but statement-level deparse recursion never routes through deparseExpr, so deeply nested trees still exhaust the C stack. Confirmed unguarded cycles: deparseRangeSubselect calls deparseSelectStmt directly at src/postgres_deparse.c:4185; the CTE cycle deparseSelectStmt -> deparseWithClause -> deparseCommonTableExpr (4011) -> deparsePreparableStmt (11726) -> deparseSelectStmt; and the CreateFunctionStmt sql_body path calling deparseStmt at 5840. deparseStmt (src/postgres_deparse.c:11841) itself has no depth check. A tree such as SELECT * FROM (SELECT * FROM (...)) thousands deep contains no a_expr at the nesting levels, so the new guard never fires and deparse crashes with a stack overflow; deparse input arrives via pg_query_deparse_protobuf, so nesting depth is not bounded by the SQL parser's own stack checks.

---

The change introduces a public API to cap recursion depth and wires PostgreSQL's check_stack_depth() mechanism into the library's traversal entry points, replacing silent depth cutoffs with PG errors in several paths.

Files	Change
src/postgres/src_backend_utils_misc_stack_depth.c	Adds stack-depth check machinery plus pg_query_set_stack_limit to configure the limit.
src/pg_query_parse.c, src/pg_query_fingerprint.c	Wrap parse/fingerprint work in PG_TRY/PG_CATCH and call check_stack_depth(); fingerprint keeps a depth>=100 cutoff on one path.
src/postgres_deparse.c	Adds depth guard to the deparse recursion (guarded in deparseExpr, not at statement level).
src/pg_query_outfuncs_json.c, src/pg_query_outfuncs_protobuf.c, src/pg_query_json_plpgsql.c	Add check_stack_depth() to node/PLpgSQL serialization recursions so deep trees raise a PG error instead of crashing.
Several files	Error paths where a PG error unwinds mid-serialization or before cleanup are not fully covered by existing handlers.

_{Reviewed commit: 64e7eda}

[devin-ai-integration]

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]

This PR adds configurable stack-depth guards (pg_query_set_stack_limit) and check_stack_depth() calls across libpg_query's parse, fingerprint, deparse, and serialization entry points to harden against stack exhaustion on deeply nested queries.

Key findings

• 🟠 Fingerprint entry points cannot catch new stack-depth error — pg_query_fingerprint.c:343
• 🟠 Protobuf parse path lacks handler for stack-depth throw — pg_query_parse.c:195
• 🟡 Guard statement-level deparse recursion, not just deparseExpr — postgres_deparse.c:11841
• 🟡 New check_stack_depth guards are inert; setter undeclared — src_backend_utils_misc_stack_depth.c:151
• 🟡 Serialize-on-parse-error path left outside PG_TRY — pg_query_parse.c:165

[tenki-reviewer]

🟡 bug · medium

New check_stack_depth guards are inert; setter undeclared

The PR adds check_stack_depth() to five recursive walkers (_fingerprintNode, _outNode in JSON and protobuf, dump_stmt/dump_expr in plpgsql JSON, deparseExpr), but stack_is_too_deep() returns false whenever stack_base_ptr == NULL (src/postgres/src_backend_utils_misc_stack_depth.c:123-127) and nothing in the library ever sets it. The only setter, pg_query_set_stack_limit (src/postgres/src_backend_utils_misc_stack_depth.c:151-159), is declared in no header and called nowhere, so embedders cannot invoke it portably; and because it writes __thread state, a limit configured on one thread leaves every other worker thread unprotected. The advertised stack-overflow protection is dead code until an undeclared, undocumented, per-thread function is called.

📋 Prompt for AI Agents

In the libpg_query repo: (1) add extern void pg_query_set_stack_limit(size_t bytes); to the public header pg_query.h alongside the other public API declarations; (2) make the stack-depth check active by default — e.g. in pg_query_enter_memory_context (src/pg_query.c), set stack_base_ptr via __builtin_frame_address(0) when it is still NULL so stack_is_too_deep() measures depth on every thread that enters a pg_query entry point, keeping pg_query_set_stack_limit as an override. Without this, the check_stack_depth() calls added in src/pg_query_fingerprint.c, src/pg_query_json_plpgsql.c, src/pg_query_outfuncs_json.c, src/pg_query_outfuncs_protobuf.c, and src/postgres_deparse.c never fire, and the new setter is unusable without a hand-written prototype.

[tenki-reviewer]

🟡 bug · medium

Serialize-on-parse-error path left outside PG_TRY

In pg_query_parse_opts the new PG_TRY only covers the result.error == NULL branch; the else branch still calls pg_query_nodes_to_json on the partial tree with no handler (src/pg_query_parse.c:167-169). check_stack_depth() in _outNode (src/pg_query_outfuncs_json.c:293) can throw for a deeply nested partial statement on exactly this error path, and the uncaught throw skips pg_query_exit_memory_context and terminates the process. Restructuring the code so the error branch runs outside the try block creates the gap; the throw can be caught as easily as the success branch.

📋 Prompt for AI Agents

In src/pg_query_parse.c, pg_query_parse_opts lines 138-170: remove the if (result.error == NULL) ... else ... split and run the pg_query_nodes_to_json/strdup/pfree sequence unconditionally inside the existing PG_TRY block, keeping the PG_CATCH that builds a PgQueryError. When a parse error already exists, the partial tree is still serialized, but a check_stack_depth()/ereport throw during serialization is caught and reported instead of longjmping out of the library unguarded.

[devin-ai-integration]

Folded into #13