From 4ad8a0671f56caa97bb38aebacf8bd5417b9a586 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Sat, 25 Jul 2026 20:55:13 +0530 Subject: [PATCH] fix: recover every declared SQL routine from unparseable PL/pgSQL (#2180) tree-sitter-sql cannot parse PL/pgSQL-only statements, and #1910's ERROR-node name recovery only covered one of the shapes that produces. Two others dropped the routine silently -- no node, no warning, exit code 0: 1. The statement is shredded into loose top-level tokens (keyword_create, keyword_function, object_reference, ..., keyword_begin) and the ERROR node holds only the offending body line, e.g. `PERFORM other_fn();` or `x := 1;`. No ERROR node contains any CREATE text, so scanning ERROR nodes finds nothing. This is what still dropped PERFORM and := after #1910. 2. The routine name is a quoted identifier -- CREATE OR REPLACE FUNCTION "public"."fn"(...) -- which the recovery's bare [\w$.]+ pattern cannot match, because it stops dead at the leading quote. Generated schema dumps quote every identifier, so whole files recovered nothing. Verified on the reported repro: the same body that drops under a quoted name is recovered fine under an unquoted one, which is why the drop looked like it depended only on the body statement. Fix mirrors the global REFERENCES fallback already in this extractor: after the tree walk, scan the raw source for every CREATE [OR REPLACE] FUNCTION/PROCEDURE and emit any routine the walk missed. Name parts accept bare or double-quoted identifiers. _add_node dedupes by node id, so routines already recovered from the tree are not emitted twice. Adds tests/fixtures/sample_plpgsql_quoted.sql -- generated-style quoted DDL whose bodies use RAISE, RAISE NOTICE, PERFORM, :=, IF..THEN and bare NULL; -- plus tests that every routine is recovered and that the file stays clean (tables before and after still extract, no duplicate ids or labels, no empty/ERROR labels, and every routine keeps its contains edge from the file node). --- graphify/extractors/sql.py | 30 +++++++++++- tests/fixtures/sample_plpgsql_quoted.sql | 62 ++++++++++++++++++++++++ tests/test_multilang.py | 44 +++++++++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/sample_plpgsql_quoted.sql diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index b9ce6fd8c..92a085779 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -200,9 +200,16 @@ def walk(node) -> None: # several statements, so scan for every CREATE in it. We deliberately # do not scan the body for FROM/JOIN references: PL/pgSQL loop # variables and locals would produce junk reads_from targets. + # + # Each name part is either a bare identifier or a double-quoted + # (delimited) one, so schema-qualified generated DDL such as + # CREATE OR REPLACE FUNCTION "public"."fn"(...) is recovered too. + # A bare [\w$.]+ stops dead at the leading quote, which silently + # dropped every quoted PL/pgSQL routine (#2180). text = _read(node) for m in re.finditer( - r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+([\w$.]+)", + r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" + r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", text, re.IGNORECASE, ): name = m.group(1) @@ -292,4 +299,25 @@ def _walk_from_refs(node, caller_nid: str, line: int) -> None: _add_edge(tbl_nid, ref_nid, "references", tbl_line) emitted.add((tbl_nid, ref_nid)) + # Global regex fallback for routines (#2180). PL/pgSQL bodies break the parse + # in more than one shape, and only the first was recovered before: + # 1. the whole CREATE lands in one ERROR node -> handled in walk() + # 2. the statement is shredded into loose top-level tokens + # (keyword_create/keyword_function/object_reference/... ) and the ERROR + # node holds only the offending body line, e.g. `PERFORM x();` or + # `x := 1;` -- so no CREATE text is inside any ERROR node at all + # 3. the name is a quoted identifier ("public"."fn"), which a bare + # [\w$.]+ pattern cannot match + # Shapes 2 and 3 silently dropped the routine: no node, no warning, exit 0. + # Scanning the raw source catches all three, and _add_node dedupes by id so + # routines already recovered from the tree are not emitted twice. + for m in re.finditer( + r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" + r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", + src_text, re.IGNORECASE, + ): + fn_name = m.group(1) + fn_line = src_text[: m.start()].count("\n") + 1 + _add_node(_make_id(stem, fn_name), f"{fn_name}()", fn_line) + return {"nodes": nodes, "edges": edges} diff --git a/tests/fixtures/sample_plpgsql_quoted.sql b/tests/fixtures/sample_plpgsql_quoted.sql new file mode 100644 index 000000000..a15d4f9c5 --- /dev/null +++ b/tests/fixtures/sample_plpgsql_quoted.sql @@ -0,0 +1,62 @@ +-- Generated-style PostgreSQL DDL (#2180): every identifier is double-quoted and +-- each body uses a PL/pgSQL-only statement, so tree-sitter-sql parses these as +-- ERROR nodes. Name recovery is the only path that can see them, and a bare +-- [\w$.]+ name pattern stops at the leading quote -- which silently dropped +-- every routine in files shaped like this. + +CREATE TABLE "public"."accounts" ( + "id" INT PRIMARY KEY, + "name" TEXT +); + +CREATE OR REPLACE FUNCTION "public"."raise_exception_fn"("p_id" "uuid") RETURNS "void" + LANGUAGE "plpgsql" SECURITY DEFINER + SET "search_path" TO 'public' + AS $$ +BEGIN + RAISE EXCEPTION 'insufficient_privilege' USING ERRCODE = '42501'; +END; +$$; + +CREATE OR REPLACE FUNCTION "public"."raise_notice_fn"() RETURNS "void" LANGUAGE "plpgsql" AS $$ +BEGIN + RAISE NOTICE 'hi'; +END; +$$; + +CREATE OR REPLACE FUNCTION "public"."perform_fn"() RETURNS "void" LANGUAGE "plpgsql" AS $$ +BEGIN + PERFORM "public"."raise_notice_fn"(); +END; +$$; + +CREATE OR REPLACE FUNCTION "public"."assign_fn"() RETURNS "void" LANGUAGE "plpgsql" AS $$ +DECLARE + "x" int; +BEGIN + x := 1; +END; +$$; + +CREATE OR REPLACE FUNCTION "public"."if_then_fn"() RETURNS "void" LANGUAGE "plpgsql" AS $$ +BEGIN + IF true THEN RAISE EXCEPTION 'x'; END IF; +END; +$$; + +CREATE OR REPLACE FUNCTION "public"."null_body_fn"() RETURNS "void" LANGUAGE "plpgsql" AS $$ +BEGIN + NULL; +END; +$$; + +CREATE PROCEDURE "public"."quoted_proc"() LANGUAGE "plpgsql" AS $$ +BEGIN + PERFORM 1; +END; +$$; + +CREATE TABLE "public"."audit_log" ( + "id" INT PRIMARY KEY, + "account_id" INT REFERENCES "public"."accounts" +); diff --git a/tests/test_multilang.py b/tests/test_multilang.py index 0cc26539f..4bac41bc4 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -542,3 +542,47 @@ def test_sql_plpgsql_clean_function_not_double_emitted(): # And nothing else is duplicated either ids = [n["id"] for n in r["nodes"]] assert len(ids) == len(set(ids)) + +def test_sql_quoted_plpgsql_routines_are_recovered(): + """#2180: quoted identifiers must not defeat the ERROR-node name recovery. + + Generated PostgreSQL DDL (Supabase-style dumps, for example) quotes every + identifier: CREATE OR REPLACE FUNCTION "public"."fn"(...). The recovery + pattern matched a bare [\\w$.]+, which stops at the leading quote, so every + routine whose body also failed to parse -- RAISE, PERFORM, :=, IF..THEN, + bare NULL; -- was dropped with no warning and exit code 0. The same body + with an *unquoted* name recovered fine, which is why the drop looked like it + depended only on the body statement. + """ + r = _extract_sql_or_skip("sample_plpgsql_quoted.sql") + labels = [n["label"] for n in r["nodes"]] + for name in ( + "raise_exception_fn", + "raise_notice_fn", + "perform_fn", + "assign_fn", + "if_then_fn", + "null_body_fn", + "quoted_proc", + ): + assert f'"public"."{name}"()' in labels, f"{name} dropped from a quoted-DDL file (#2180)" + +def test_sql_quoted_plpgsql_file_stays_clean(): + """The #2180 recovery must not add junk, duplicates, or drop the tables.""" + r = _extract_sql_or_skip("sample_plpgsql_quoted.sql") + labels = [n["label"] for n in r["nodes"]] + # Tables before and after the unparseable routines still extract. + assert any("accounts" in l for l in labels) + assert any("audit_log" in l for l in labels) + # No empty or ERROR labels leaked out of the recovery. + for l in labels: + assert l, "empty node label" + assert l != "ERROR" + # Nothing emitted twice (a routine must not come from both paths). + ids = [n["id"] for n in r["nodes"]] + assert len(ids) == len(set(ids)), "duplicate node ids" + assert len(labels) == len(set(labels)), f"duplicate labels: {labels}" + # Every recovered routine is reachable from the file node. + contains_targets = {e["target"] for e in r["edges"] if e["relation"] == "contains"} + fn_ids = {n["id"] for n in r["nodes"] if n["label"].endswith("()")} + assert fn_ids <= contains_targets