[pyramation]

Summary

pg_query_scan never checked its three mallocs: the token pointer array, each PgQuery__ScanToken, and result.pbuf.data. When one fails, the next write goes through NULL:

• natively this is a segfault;
• in the WASM build (1 GiB heap) pg_query__scan_result__pack writes the protobuf at address 0 and overwrites static data, so the module later aborts with Assertion failed: <protobuf bytes> or table index is out of bounds. This shows up at about 50 MB of short statements.

Now each failure frees what was allocated so far and raises ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))). The existing PG_CATCH turns that into result.error, with the same message palloc uses. On a pbuf.data failure, pbuf.len is reset to 0.

Test

test/scan.c adds test_scan_out_of_memory() (Linux only, skipped under USE_VALGRIND). It caps RLIMIT_AS at the current size plus 128 MiB, scans "SELECT 1;\n" × 2,000,000 (6M tokens), and expects result.error->message == "out of memory".

• Before: test/scan segfaults (exit 139).
• After: make test exits 0.
• make test VALGRIND=1: test/scan and every other test is clean except test/parse_plpgsql. That one still reports a 192-byte still-reachable block from pg_query_plpgsql_record_alias (pg_query_json_plpgsql.c:826). This predates the PR: it is red on 18-constructive already and is fixed in Make check_stack_depth() effective for embedders; guard recursive output paths #10.

Not changed: pg_query_nodes_to_protobuf has the same unchecked malloc for protobuf.data, but it runs outside PG_TRY (it is called from pg_query_parse_protobuf_opts), so ereport would not be caught there. It needs a separate error path.

Merge with a merge commit (not squash): constructive-io/libpg-query-node#188 pins this branch's head commit 8076f09.

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

[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]

Review complete. 🔴 1 critical · 🟡 1 medium

💬 Inline comments (1)

• 🟡 Error paths leak the token array and yyscanner — pg_query_scan.c:104

📍 Findings outside the diff (1) — 🔴 1 critical — defects on lines GitHub can't attach comments to

🔴 Critical — Missing semicolon breaks compilation of pg_query_scan.c · pg_query_scan.c:63 · unchanged line

// src/pg_query_scan.c
63	    error->message = strdup("Failed to open pipe, too many open file descriptors")

The error->message = strdup(...) statement has no terminating semicolon (src/pg_query_scan.c:63), so the next line result.error = error; starts a new statement while the previous one is unterminated. Any C compiler rejects this, which fails the build of the entire scan module introduced by this PR.

🧹 Nitpicks (2) — 🟢 2 low

• 🟢 Pipe-failure return skips memory context exit (pg_query_scan.c:60) — When pipe() fails, the function returns the error result directly (src/pg_query_scan.c:67) without calling pg_query_exit_memory_context(ctx), which is only reached at line 180 on the normal path.
• 🟢 OOM test ignores setrlimit failure and flaky headroom (scan.c:44) — The test computes the soft RLIMIT_AS limit as current usage plus a hardcoded 128 MB and never checks the getrlimit/setrlimit return values (test/scan.c:44-49).

---

This PR introduces a tokenizing entry point in src/pg_query_scan.c that runs core_yylex in its own memory context, packs the resulting tokens into a protobuf PgQuery__ScanToken list, and returns them via PgQueryScanResult, along with a new out-of-memory test in test/scan.c that applies an RLIMIT_AS limit before scanning a 2M-statement input.

Files	Change
src/pg_query_scan.c	New scan API: sets up a memory context, opens a flex scanner, collects tokens, and serializes them with the existing protobuf packing helpers.
test/scan.c	New test_scan_out_of_memory case that lowers RLIMIT_AS to force allocation failure during scanning and asserts an error is reported.

Risk: high — the module does not compile as written, and the lexer error paths leak malloc-allocated resources.

_{Reviewed commit: 8076f09}

[tenki-reviewer]

Adds a new pg_query_scan API that tokenizes SQL with the core scanner and an OOM regression test, but a missing semicolon currently breaks compilation and error paths leak scanner resources.

Key findings

• 🔴 Missing semicolon breaks compilation of pg_query_scan.c — pg_query_scan.c:63
• 🟡 Error paths leak the token array and yyscanner — pg_query_scan.c:104

[tenki-reviewer]

🟡 bug · medium

Error paths leak the token array and yyscanner

When core_yylex raises an error mid-loop — invalid byte sequence, unterminated string, or overlong identifier — the ereport(ERROR) longjmps to PG_CATCH(), which never frees the partially built output_tokens array or finishes the second yyscanner (src/pg_query_scan.c:104-109). In the per-token OOM path the tokens are freed but the flex scanner opened at line 93 is likewise left un-finalized. Because flex scanner buffers and the token array are plain malloc allocations outside the PostgreSQL memory context, pg_query_exit_memory_context cannot reclaim them, so every failed scan leaks the token array plus scanner buffers.

📋 Prompt for AI Agents

In src/pg_query_scan.c, move the PgQuery__ScanToken **output_tokens declaration above PG_TRY() and initialize it to NULL. In the PG_CATCH() block (around lines 151-170), after MemoryContextSwitchTo(parse_context), add a guarded cleanup: if output_tokens != NULL, call free_scan_tokens(output_tokens, token_count) and set output_tokens = NULL; if yyscanner != NULL, call scanner_finish(yyscanner) and set yyscanner = NULL. On the success path set output_tokens = NULL and yyscanner = NULL right after the existing free_scan_tokens and scanner_finish calls (lines 127 and 142) so the catch-block cleanup cannot double-free. This closes the leak where an ereport(ERROR) from core_yylex or the per-token malloc failure longjmps past cleanup, leaking the malloc-allocated token array and the flex scanner buffers.

[devin-ai-integration]

Folded into #13