diff --git a/graphify/extractors/sql.py b/graphify/extractors/sql.py index a5dc18c368..675f89ec52 100644 --- a/graphify/extractors/sql.py +++ b/graphify/extractors/sql.py @@ -6,6 +6,12 @@ from pathlib import Path from graphify.extractors.base import _file_stem, _make_id +# One dot-separated part of a table/view/routine name recovered from an ERROR +# node's raw text: backtick-quoted (also matches a debracketed T-SQL name, +# #2718), double-quoted (Postgres/ANSI, #2180), or bare. Shared by the +# regex-fallback sites below so they can't drift apart. +_SQL_NAME_PART = r'(?:`[^`\n]+`|"[^"\n]+"|[\w$]+)' + def _norm_ident(name: str) -> str: """Normalize a SQL identifier for name-based reference resolution. @@ -27,6 +33,112 @@ def _norm_ident(name: str) -> str: return ".".join(parts) +def _debracket_tsql(source: bytes) -> tuple[bytes, bool]: + """Rewrite T-SQL `[bracket]`-quoted identifiers to `` `backtick` ``-quoted ones. + + tree-sitter-sql has no grammar token for T-SQL bracket quoting: each `[` + and `]` lands as its own one-byte ERROR node, one character short of the + real pair. That shifts every subsequent token in the statement by one + byte, which corrupts the object_reference text — `[dbo].[Customer]` reads + back as `dbo].[Customer` — so the node label keeps a stray bracket + fragment on each side of the dot instead of the real name (#2712). + + Backtick quoting parses as a clean atomic `identifier` token (the + grammar's MySQL-dialect support) and is not otherwise valid T-SQL syntax, + so substituting one for the other before parsing sidesteps the grammar + gap rather than trying to patch the corrupted text after the fact. `]]` + is T-SQL's escape for a literal `]` inside a bracketed name; it is + unescaped here and re-escaped as a doubled backtick if needed. + + Only scans outside `'...'` string literals, `--` line comments, and + `/* */` block comments, so a literal `[` in either is left untouched. A + `[...]` span is left alone (not substituted) when it is unterminated, + empty, spans a newline, or its content is purely numeric — those are + Postgres/MySQL array-type syntax (`int[]`, `numeric(10)[3]`), never a + valid T-SQL identifier, and must not be corrupted. + + Returns ``(source, False)`` unchanged if nothing qualified. + """ + out = bytearray() + i, n = 0, len(source) + changed = False + while i < n: + c = source[i] + if c == ord("-") and i + 1 < n and source[i + 1] == ord("-"): + j = source.find(b"\n", i) + j = n if j == -1 else j + out += source[i:j] + i = j + continue + if c == ord("/") and i + 1 < n and source[i + 1] == ord("*"): + j = source.find(b"*/", i + 2) + j = n if j == -1 else j + 2 + out += source[i:j] + i = j + continue + if c == ord("'"): + j = i + 1 + while j < n: + if source[j] == ord("'"): + if j + 1 < n and source[j + 1] == ord("'"): + j += 2 + continue + j += 1 + break + j += 1 + out += source[i:j] + i = j + continue + if c == ord("["): + j = i + 1 + content = bytearray() + terminated = False + while j < n: + if source[j] == ord("]"): + if j + 1 < n and source[j + 1] == ord("]"): + content.append(ord("]")) + j += 2 + continue + j += 1 + terminated = True + break + if source[j] == ord("\n"): + break + content.append(source[j]) + j += 1 + if not terminated or not content or content.strip().isdigit(): + out.append(c) + i += 1 + continue + changed = True + out.append(0x60) + out += bytes(content).replace(b"`", b"``") + out.append(0x60) + i = j + continue + out.append(c) + i += 1 + return bytes(out), changed + + +def _strip_backtick_parts(name: str) -> str: + """Undo `_debracket_tsql`'s synthetic backtick-quoting for a display label. + + Splits on `.` and strips a matching pair of backticks (unescaping a + doubled backtick back to one) from each part. Only ever called on source + that `_debracket_tsql` has already confirmed contains no genuine + backtick, so every backtick encountered here is one it introduced; + double-quoted (ANSI/Postgres) identifiers are untouched either way. + """ + parts = [] + for part in name.split("."): + p = part.strip() + if len(p) >= 2 and p[0] == "`" and p[-1] == "`": + p = p[1:-1].replace("``", "`") + parts.append(p) + return ".".join(parts) + + def extract_sql(path: Path, content: str | bytes | None = None) -> dict: """Extract tables, views, functions, and relationships from .sql files via tree-sitter.""" try: @@ -53,6 +165,16 @@ def extract_sql(path: Path, content: str | bytes | None = None) -> dict: else content if content is not None else path.read_bytes() ) + # A backtick already in the source means it's either genuinely + # backtick-quoted (MySQL dialect) or mixes dialects in a way this + # module cannot safely disambiguate; skip debracketing rather than + # risk misreading a real backtick as one we introduced (#2712). + debracketed = False + if b"`" not in source: + _new_source, _changed = _debracket_tsql(source) + if _changed: + source = _new_source + debracketed = True tree = parser.parse(source) root = tree.root_node except Exception as e: @@ -71,10 +193,18 @@ def extract_sql(path: Path, content: str | bytes | None = None) -> dict: def _read(n) -> str: return source[n.start_byte:n.end_byte].decode("utf-8", errors="replace") + def _clean_name(name: str) -> str: + """Strip synthetic backtick-quoting from a name, when this file was debracketed.""" + return _strip_backtick_parts(name) if debracketed else name + + def _ident(n) -> str: + """Read an identifier/object_reference node as a clean display name.""" + return _clean_name(_read(n)) + def _obj_name(n) -> str | None: for c in n.children: if c.type == "object_reference": - return _read(c) + return _ident(c) return None def _add_node(nid: str, label: str, line: int) -> None: @@ -138,7 +268,7 @@ def walk(node) -> None: if cc.type == "keyword_references": found_ref = True elif found_ref and cc.type == "object_reference": - ref_name = _read(cc) + ref_name = _ident(cc) break if ref_name: ref_nid = table_nids.get(_norm_ident(ref_name)) or _ref_stub(ref_name) @@ -155,7 +285,7 @@ def walk(node) -> None: if cc.type == "keyword_references": found_ref = True elif found_ref and cc.type == "object_reference": - ref_name = _read(cc) + ref_name = _ident(cc) break if ref_name: ref_nid = table_nids.get(_norm_ident(ref_name)) or _ref_stub(ref_name) @@ -216,7 +346,7 @@ def walk(node) -> None: if ccc.type == "keyword_references": found_ref = True elif found_ref and ccc.type == "object_reference": - ref_name = _read(ccc) + ref_name = _ident(ccc) break if ref_name: ref_nid = (table_nids.get(_norm_ident(ref_name)) @@ -232,11 +362,11 @@ def walk(node) -> None: if c.type == "keyword_trigger": after_trigger = True elif after_trigger and not trig_name and c.type == "object_reference": - trig_name = _read(c) + trig_name = _ident(c) elif c.type == "keyword_for": after_for = True elif after_for and not tbl_name and c.type == "object_reference": - tbl_name = _read(c) + tbl_name = _ident(c) if trig_name: trig_nid = _make_id(stem, trig_name) _add_node(trig_nid, trig_name, line) @@ -254,19 +384,24 @@ def walk(node) -> None: # 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). + # Each name part is a bare identifier, a double-quoted (delimited) + # one, or a backtick-quoted 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). + # The backtick alternative also recovers a bracket-quoted T-SQL + # name after `_debracket_tsql` has rewritten it (#2718) — this + # ERROR-node fallback is the ONLY path that ever sees a T-SQL + # CREATE PROCEDURE/FUNCTION, since this grammar has no rule at all + # for its `AS BEGIN ... END` body, bracketed name or not. text = _read(node) for m in re.finditer( r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" r"(?:IF\s+NOT\s+EXISTS\s+)?" - r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", + rf"({_SQL_NAME_PART}(?:\s*\.\s*{_SQL_NAME_PART})*)", text, re.IGNORECASE, ): - name = m.group(1) + name = _clean_name(m.group(1)) m_line = line + text[: m.start()].count("\n") nid = _make_id(stem, name) _add_node(nid, f"{name}()", m_line) @@ -357,7 +492,7 @@ def _walk_from_refs(node, caller_nid: str, line: int, if c.type == "relation": for cc in c.children: if cc.type == "object_reference": - tbl = _read(cc) + tbl = _ident(cc) if _norm_ident(tbl) in cte_names: continue tbl_nid = table_nids.get(_norm_ident(tbl)) or _ref_stub(tbl) @@ -444,10 +579,10 @@ def _collect_defined_names(node) -> None: for m in re.finditer( r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:FUNCTION|PROCEDURE)\s+" r"(?:IF\s+NOT\s+EXISTS\s+)?" - r"((?:\"[^\"\n]+\"|[\w$]+)(?:\s*\.\s*(?:\"[^\"\n]+\"|[\w$]+))*)", + rf"({_SQL_NAME_PART}(?:\s*\.\s*{_SQL_NAME_PART})*)", src_text, re.IGNORECASE, ): - fn_name = m.group(1) + fn_name = _clean_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) diff --git a/tests/test_multilang.py b/tests/test_multilang.py index cb390eebcc..f6f7fa4091 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -672,6 +672,157 @@ def test_sql_schema_qualified_alter_fk(): assert e["source"] in node_ids, f"dangling source: {e['source']}" assert e["target"] in node_ids, f"dangling target: {e['target']}" +def test_sql_tsql_bracket_identifiers_produce_clean_labels(tmp_path): + """#2712: [dbo].[Alpha] must label as dbo.Alpha, not the delimiter-mangled + `dbo].[Alpha` tree-sitter-sql's grammar produces for bracket quoting (it has + no token for `[...]`, so each bracket lands as its own one-byte ERROR node, + one character short of the real pair).""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "schema.sql" + p.write_text( + "CREATE TABLE [dbo].[Alpha] (\n" + " [Id] INT NOT NULL PRIMARY KEY\n" + ");\n" + "GO\n" + "CREATE TABLE [dbo].[Beta] (\n" + " [Id] INT NOT NULL PRIMARY KEY\n" + ");\n" + "GO\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"]] + assert "dbo.Alpha" in labels, f"got {labels}" + assert "dbo.Beta" in labels, f"got {labels}" + assert not any("]" in l or "[" in l for l in labels), ( + f"a bracket fragment leaked into a label: {labels}" + ) + +def test_sql_tsql_bracket_reference_resolves_by_clean_name(tmp_path): + """#2712: a bracket-quoted FOREIGN KEY ... REFERENCES [dbo].[Alpha] must + resolve onto the real Alpha table node, not dangle or mint a stub keyed by + the mangled `Alpha` text.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "schema.sql" + p.write_text( + "CREATE TABLE [dbo].[Alpha] ([Id] INT NOT NULL PRIMARY KEY);\n" + "GO\n" + "CREATE TABLE [dbo].[Beta] (\n" + " [Id] INT NOT NULL PRIMARY KEY,\n" + " [AlphaId] INT NOT NULL,\n" + " CONSTRAINT [FK_Beta_Alpha] FOREIGN KEY ([AlphaId])\n" + " REFERENCES [dbo].[Alpha] ([Id])\n" + ");\n" + "GO\n" + ) + r = extract_sql(p) + nid = {n["label"]: n["id"] for n in r["nodes"]} + assert "dbo.Alpha" in nid and "dbo.Beta" in nid + refs = {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == "references"} + assert (nid["dbo.Beta"], nid["dbo.Alpha"]) in refs, f"got {refs}" + +def test_sql_tsql_bracket_debracketing_does_not_corrupt_array_types(tmp_path): + """#2712 follow-up: the bracket->backtick rewrite must not misfire on + Postgres/MySQL array-type syntax (`text[]`, `numeric(10)[3]`), which uses + `[...]` for something other than a T-SQL quoted identifier.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "schema.sql" + p.write_text("CREATE TABLE t (id INT, tags text[], scores numeric(10,2)[3]);\n") + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"]] + assert any(l == "t" for l in labels), f"table extraction broke on array types: {labels}" + for l in labels: + assert "`" not in l, f"a synthetic backtick leaked into a label: {labels}" + +def test_sql_tsql_bracketed_fk_does_not_drop_child_table_or_fabricate_self_loop(tmp_path): + """#2713: a bracket-quoted FOREIGN KEY ... REFERENCES clause used to confuse + the parser badly enough that the whole child table (Invoice) — FK + constraint included — landed as bogus nested content inside the PARENT + table's (Customer) own subtree. That dropped Invoice from the graph + entirely and fabricated a Customer -> Customer self-referencing edge + tagged EXTRACTED (highest confidence) where no such reference exists in + the source. Fixed as a side effect of #2712's debracketing: with clean + identifier tokens the two CREATE TABLE statements parse as separate + top-level statements again.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "s.sql" + p.write_text( + "CREATE TABLE [dbo].[Customer] (\n" + " [CustomerId] INT NOT NULL PRIMARY KEY,\n" + " [Name] NVARCHAR(100) NULL\n" + ");\n" + "GO\n" + "\n" + "CREATE TABLE [dbo].[Invoice] (\n" + " [InvoiceId] INT NOT NULL PRIMARY KEY,\n" + " [CustomerId] INT NOT NULL,\n" + " CONSTRAINT [FK_Invoice_Customer] FOREIGN KEY ([CustomerId])\n" + " REFERENCES [dbo].[Customer] ([CustomerId])\n" + ");\n" + "GO\n" + ) + r = extract_sql(p) + nid = {n["label"]: n["id"] for n in r["nodes"]} + assert "dbo.Customer" in nid, "Customer table missing from the graph" + assert "dbo.Invoice" in nid, "Invoice table was dropped from the graph (#2713)" + + refs = {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == "references"} + assert (nid["dbo.Customer"], nid["dbo.Customer"]) not in refs, ( + "fabricated Customer -> Customer self-loop present (#2713)" + ) + assert (nid["dbo.Invoice"], nid["dbo.Customer"]) in refs, ( + f"expected Invoice -> Customer reference edge, got {refs}" + ) + +def test_sql_tsql_bracketed_procedure_and_function_names_are_extracted(tmp_path): + """#2718: a CREATE PROCEDURE/FUNCTION whose NAME is bracket-quoted used to + produce no node at all (0 of 845 procedures on a real SSMS-scripted dump). + This grammar has no rule for T-SQL's `AS BEGIN ... END` routine body at + all — bracketed name or not — so recovery always goes through the + ERROR-node regex fallback; that fallback matched a bare or double-quoted + name but not a bracket-quoted one, so it silently dropped every bracketed + routine with no warning and exit 0.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "schema.sql" + p.write_text( + "CREATE TABLE dbo.Customer (Id INT NOT NULL PRIMARY KEY);\n" + "GO\n" + "CREATE PROCEDURE [dbo].[GetCustomer] @Id INT\n" + "AS\n" + "BEGIN\n" + " SELECT Id FROM dbo.Customer WHERE Id = @Id;\n" + "END\n" + "GO\n" + "CREATE FUNCTION [dbo].[CustomerName] (@Id INT)\n" + "RETURNS INT\n" + "AS\n" + "BEGIN\n" + " RETURN (SELECT Id FROM dbo.Customer WHERE Id = @Id);\n" + "END\n" + "GO\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"]] + assert "dbo.GetCustomer()" in labels, f"bracketed procedure dropped: {labels}" + assert "dbo.CustomerName()" in labels, f"bracketed function dropped: {labels}" + +def test_sql_tsql_bracketed_procedure_no_schema_is_extracted(tmp_path): + """#2718: a schema-less bracketed name ([GetCustomer], no `].[`) must also + recover — the defect was the bracket quoting on the callable's own name, + independent of the #2712 `].[ ` label-mangling trigger.""" + pytest.importorskip("tree_sitter_sql") + p = tmp_path / "schema.sql" + p.write_text( + "CREATE PROCEDURE [GetCustomer] @Id INT\n" + "AS\n" + "BEGIN\n" + " SELECT 1;\n" + "END\n" + "GO\n" + ) + r = extract_sql(p) + labels = [n["label"] for n in r["nodes"]] + assert "GetCustomer()" in labels, f"got {labels}" + def test_sql_plpgsql_functions_survive_parse_errors(): """PL/pgSQL bodies make tree-sitter-sql emit ERROR nodes; the functions must still be extracted (#1910), without cascading into later statements."""