fix(plugin-oracle): run PL/SQL blocks and units whole, with the terminator Oracle needs - #2988
Merged
Merged
Conversation
…nator Oracle needs
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2984
The bug
The editor split every Oracle script at every
;, soBEGIN DBMS_OUTPUT.PUT_LINE('x'); END;reached Oracle as two fragments and failed with ORA-06550 / PLS-00103. That was one symptom of a larger one:BEGINonly opened a block afterCREATE,ALTER,REPLACEorDECLARE, andDECLAREitself opened nothing, so anonymous blocks,DECLAREblocks, procedures with a declaration section, package specifications (noBEGINat all) and compound triggers were all cut at an inner;.;stripped. A PL/SQL unit needs it. Measured on Oracle 23ai through the app's ownOracleCoreConnection: a block sent without it fails PLS-00103, and aCREATE PROCEDURE,PACKAGEorTRIGGERsent without it is stored INVALID while the statement reports success. oracle-nio drops the compile warning, so every PL/SQL unit created from the editor was broken with a green result.;, SQL import had no block tracking at all, and folding disagreed with the run controls about where a unit ends.The fix
One statement-boundary grammar that every reader feeds, instead of each reader guessing:
SQLStatementBoundaryTrackingdecides where a statement ends and whether its;belongs to it. It is forward only, so the streaming SQL import parser can feed it across 64 KiB chunk boundaries.PLSQLUnitTrackeris Oracle's grammar, modelled on SQL*Plus's own rule for entering PL/SQL mode: anonymous blocks, stored units (PROCEDURE,FUNCTION,PACKAGE [BODY],TYPE [BODY],TRIGGER,LIBRARY),WITH FUNCTIONqueries, andCREATE JAVA/MLE MODULE/WRAPPEDbodies that only a/line ends. It counts the constructs oneENDcloses (aBEGINcontinues a declaration section rather than nesting), treatsEND IF/END LOOP/END CASE,$IF…$END, labels, member names after.and call specs correctly, and keeps a unit's final;except after aCALL-bodied trigger, which Oracle stores INVALID with one.SQLRoutineBodyTrackeris the existing rule for every other dialect, moved out of the scanner unchanged except forEND CASE, which used to reopen a block and swallow everything after a MySQL procedure containing aCASEstatement.SqlDialect.oracle(PluginKit, additive, reuses the pending kit 33) carries Oracle's lexical rules:q'[…]'literals, no backslash escapes,/terminator lines.Wired through the editor (run at cursor, selection, Run All, gutter, navigation), folding, SQL import, the external-client bridge and Compare & Sync (Oracle only).
Keeping a block whole must not weaken the execution gate, which tiers by leading keyword:
WITHclause declares a function or procedure, is classified as server-side code execution (parity with PostgreSQLDO), so MCP and the AI assistant refuse it and a Read-Only connection treats it as a write. Its tier is the worst ofDROP/TRUNCATEin its body and the literals it passes toEXECUTE IMMEDIATE,DBMS_SQL.PARSEandEXEC_DDL_STATEMENT, read by Oracle's lexing rules (no backslash escapes,q'[…]'), and aDELETEwithoutWHEREinside it raises the dangerous-query warning (a collection'sv.DELETEdoes not).DECLAREblock refreshes the sidebar like aBEGINblock; OracleBEGINis never read as a transaction.:NEWin a trigger body no longer opens the parameter panel or gets rewritten into a placeholder.The Oracle plugin now reads
ALL_ERRORSafter aCREATEof a PL/SQL unit and fails with eachline:column PLS-…instead of reporting success, and reports 0 rows affected for a block instead of oracle-nio's 1.Evidence
Every corpus script, split by the real scanner sources from
mainand from this branch, each statement sent to Oracle 23ai Free throughOracleCoreConnection:(The one remaining failure is an
INSERTthat fired a trigger my earlier probing had left INVALID on that table.)scripts/check-oracle-plsql-terminators.shre-measures the keep/strip rule against a live server; on 23ai every shape agrees:Before / After
Running the block from the issue with
Cmd+Enter:BEGIN\n DBMS_OUTPUT.PUT_LINE('Hello from PL/SQL')ORA-06550 … PLS-00103: Encountered the symbol "end-of-file"BEGIN\n DBMS_OUTPUT.PUT_LINE('Hello from PL/SQL');\nEND;No screenshots: the only new text on screen is the compilation-error message, shown in the existing error banner, and the change a reviewer needs to see is the statement text, which the measured run above covers statement by statement.
Review
Codex was at its usage limit, so the independent passes were the
code-reviewandsecurity-reviewskills.code-reviewfound four defects, all fixed with regression tests: the block classifier stripped literals with the generic lexer, so'C:\temp\'orq'[it's]'could hide aDELETE;v.DELETEon a collection raised the dangerous-query warning; the caret on a/line ran nothing; and the import parser dropped a file's last character when it waited on lookahead at end of file. That last one predates this change for quotes and dashes, and the q-quote check widened it, so the end of input is now processed rather than left in the buffer.security-reviewfound one high-severity regression before it shipped: keepingWITH FUNCTION … END; SELECT …whole let the classifier tier it a safe read, so a read-only MCP client could runEXECUTE IMMEDIATE 'DROP …'inside it. It is now classified like an anonymous block, pinned byQueryClassifierPLSQLTests.Tests
PLSQLScriptCorpus: 21 Oracle scripts with the exact text each statement is sent as, every one measured VALID or run on 23ai. Pinned by the scanner (SQLStatementPLSQLSplittingTests), SQL import at every possible chunk-boundary position (SQLFileParserPLSQLTests) and fold/run-control agreement (SQLFoldScannerTests).QueryClassifierPLSQLTests: tiers, dynamic SQL, external gate refusal, catalog refresh, batch policy, bridge text.OraclePLSQLUnitTests(package): header parsing,ALL_ERRORSquery escaping, message format.END CASEregression, q-quote coverage inSqlLexer,SQLNonCodeSpan,SQLTokenCursor, Oracle Compare & Sync terminators.No UI automation: CI has no Oracle server, so a run of a PL/SQL block cannot be driven deterministically there. The live measurements above stand in for it.
Not in this PR
Reported separately rather than changed here: Dameng still maps to the generic dialect (no DM8 to measure against, and its backslash escaping is configurable); a column named
beginmergesCREATE TABLE …; DROP …into one statement on PostgreSQL/MySQL/SQLite; SQL import for other dialects still has no routine-body tracking; the iOS Oracle editor sends raw text; SQL*Plus client commands (SET SERVEROUTPUT,EXEC,SHOW ERRORS) are not interpreted. DBMS_OUTPUT display follows in a stacked PR.