Skip to content

plpgsql-parser: walkSql silently returns aborted:false with zero visits when the PL/pgSQL body fails to parse #351

Description

@mib00038

Summary

walkSql (packages/plpgsql-parser) returns { aborted: false } with zero PLpgSQL_* node visits when a LANGUAGE plpgsql function body fails to parse. A syntactically broken body is indistinguishable from a clean body with "no matching constructs". The doc comment on walkSql says unparseable input is reported as an abort; that holds only for the outer SQL parse, not the PL/pgSQL body.

Observed on plpgsql-parser@18.5.8 (libpg-query 18.1.4 WASM), Node 22.23.2.

Where

  • src/parse.ts (~lines 101-103): any throw from parsePlPgSQLSync / hydratePlpgsqlAst is caught and the function is returned with plpgsql: null; the item is then pushed as a plain kind: 'stmt'.
  • walk.ts (~lines 296-297): if (!hydrated) continue; skips the body silently.
  • hydratePlpgsqlAst's errors[] is never consulted by walkSql / walk.
  • traverse.ts (~199-201): empty / whitespace-only input also returns { aborted: false }.

Reproducer

import { loadModule, walkSql, parseSql } from 'plpgsql-parser';
await loadModule();

const broken = `
CREATE OR REPLACE FUNCTION public.f() RETURNS void
LANGUAGE plpgsql AS $$
BEGIN
  UPDATE connectors SET instance_id = ;   -- syntax error inside the body
END;
$$;`;

let plpgsqlNodes = 0;
const result = walkSql(broken, (node) => { if (String(node?.type ?? '').startsWith('PLpgSQL_')) plpgsqlNodes++; });
console.log(result);          // { aborted: false }
console.log(plpgsqlNodes);    // 0

const parsed = parseSql(broken);
console.log(parsed.functions.length);  // 0  (declared plpgsql functions: 1)

Compare: a valid body with BEGIN END; visits 5 PLpgSQL_* nodes, and an outer-SQL syntax error (e.g. a typo before LANGUAGE) correctly yields { aborted: true, reason }.

Expected

One of:

  1. walkSql returns { aborted: true, reason } when a declared plpgsql function fails hydration (fail-closed by default), or
  2. the result exposes the hydration failures (e.g. errors[] / unhydrated: [...]) so callers can fail closed, optionally behind a strict option if backward compatibility matters.

Why it matters

Consumers using the walker as a structural assertion over function bodies (e.g. "this body contains no UPDATE on table X") get a false green on a broken body. We currently wrap it with a caller-side guard (declared function count vs hydrated count, errors.length, and zero-PLpgSQL_*-visit check); happy to open a PR with the fix + a test from this reproducer if you'd like — say which of (1)/(2) you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions