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