Skip to content

fix(sql): accept Oracle [NON]EDITIONABLE in routine and trigger recovery (#5167) - #5183

Closed
Yi-111-a wants to merge 1 commit into
Graphify-Labs:v8from
Yi-111-a:fix/sql-editionable-routines
Closed

Yi-111-a wants to merge 1 commit into
Graphify-Labs:v8from
Yi-111-a:fix/sql-editionable-routines

Conversation

@Yi-111-a

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #5167. Oracle's default DDL export (DBMS_METADATA.GET_DDL and most IDE exports) writes CREATE OR REPLACE EDITIONABLE PROCEDURE|FUNCTION|TRIGGER (or NONEDITIONABLE). Those statements land in an ERROR node, and the recovery regexes _ROUTINE_RECOVERY_RX and _TRIGGER_RECOVERY_RX only allowed CREATE [OR REPLACE|ALTER] directly followed by the object kind, so the routine or trigger was never recovered and the file came back with only its file node.

Both regexes now accept an optional (?:NON)?EDITIONABLE\s+ between CREATE [OR REPLACE] and the object kind, which is the change proposed in the issue.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

Invariant: adding EDITIONABLE / NONEDITIONABLE to a routine or trigger DDL does not change the extracted nodes or edges.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

On v8 with tree-sitter-sql 0.3.13, CREATE OR REPLACE NONEDITIONABLE FUNCTION hr.f ... and CREATE OR REPLACE EDITIONABLE TRIGGER hr.trg_emp ... ON hr.emp ... returned only the file node. With this change they produce hr.f() (with its reads_from hr_emp) and hr.trg_emp (with triggers hr_emp), identical to the same files without the keyword.

tests/test_sql_oracle_editionable.py checks that procedure, function and trigger DDL with EDITIONABLE, NONEDITIONABLE and lower-case editionable extract exactly the same graph as the plain form, that the trigger keeps its subject table, and that a table literally named editionable does not turn into a routine. The function and trigger cases fail without the fix.

Limitation: with 0.3.13 the EDITIONABLE PROCEDURE ... IS case from the issue already recovered the procedure on v8 for me, so this mainly changes functions and triggers here; the test still pins the procedure case. The missing writes_to hr_emp_copy for INSERT INTO ... SELECT * inside the body happens with or without the keyword and is left alone. The Firebird fb_proc_or_trigger path is untouched since EDITIONABLE is Oracle-only. This changes per-file extractor output, so it may want to ride along with the next cache schema bump.

How was this tested?

# tree-sitter-sql 0.3.13
uv run pytest -q tests/test_sql_oracle_editionable.py
# 11 passed (7 fail on v8 without the fix)
uv run pytest -q tests -k "sql or SQL"
# 111 passed, 6 failed; the same 6 fail on v8 without this change (0.3.13 T-SQL / recovery regressions)

@Yi-111-a
Yi-111-a requested a review from safishamsi as a code owner November 12, 2026 16:51
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @Yi-111-a. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@github-actions github-actions Bot added the contributor Pull request from a returning community contributor label Nov 12, 2026

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.

Not checked: tests were not run; no formal proof of the changed code.


Graphify review — findings

Accepts Oracle's optional EDITIONABLE/NONEDITIONABLE keyword in the SQL extractor's routine and trigger recovery patterns, so CREATE OR REPLACE [NON]EDITIONABLE PROCEDURE|FUNCTION|TRIGGER (the default DBMS_METADATA.GET_DDL output) yields the same routine, trigger, and subject-table edges as plain DDL. Previously such files recovered no routine at all (#5167). The keyword is only matched between CREATE [OR REPLACE] and the object kind, so a table or column named editionable still doesn't mint a phantom routine; new tests cover each kind, mixed case, and that negative case.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 3165 functions depend on the 36 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract_sql() — 46 callers, 13 callees

Verification — 3165 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1218 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

7 of 560 test file(s) selected (1%) via static blast radius.

  • tests/test_extract.py — impact
  • tests/test_multilang.py — impact
  • tests/test_pg_introspect.py — impact
  • tests/test_python_member_call_language_gate.py — impact
  • tests/test_sql_go_batches.py — impact
  • tests/test_sql_oracle_editionable.py — impact, changed-test
  • tests/test_sql_tsql_procedures.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

· 1 more finding(s) on lines outside this diff (see the check run).

@safishamsi

Copy link
Copy Markdown
Member

Landed on v8 as 131d302 via authorship-preserving cherry-pick, released in graphify 0.9.108. Thanks @Yi-111-a.

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

Labels

contributor Pull request from a returning community contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: SQL: CREATE OR REPLACE EDITIONABLE/NONEDITIONABLE PROCEDURE|FUNCTION|TRIGGER yields only a file node (0 edges)

2 participants