fix(plugin-oracle): run PL/SQL blocks and units whole, with the terminator Oracle needs

CHANGELOG.md

@@ -38,6 +38,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Oracle PL/SQL blocks split at their inner semicolons and sent as fragments, failing with PLS-00103. (#2984)
+- Oracle procedures, packages and triggers created from the editor stored INVALID while the run reported success.
+- SQL*Plus `/` lines, `q'[…]'` literals and backslashes in strings misread in Oracle scripts.
+- `:NEW` and `:OLD` in an Oracle trigger body opening the parameter panel.
+- 1 row affected reported for every Oracle PL/SQL block.
+- MySQL procedures with a `CASE` statement swallowing the statements after them in the editor.
 - Icon-only buttons announced as nothing by VoiceOver across the data grid, row inspector, editor find bar, filter bar, structure, dashboard and settings.
 - Status icons that carried a result only as a symbol and a colour, silent to VoiceOver, in the AWS and app import steps and the plugin lists.
 - Foreign key picker rows that could only be chosen with a mouse.

Packages/TableProOracle/Sources/TableProOracleCore/OraclePLSQLUnit.swift

@@ -0,0 +1,212 @@
+import Foundation
+
+/// A stored PL/SQL unit a `CREATE` statement defines, read from the statement's header.
+///
+/// Oracle answers a `CREATE PROCEDURE` whose body does not compile with success: the object is stored INVALID and the
+/// failure travels only as a warning flag, which the driver drops. The errors are in `ALL_ERRORS`, keyed by owner,
+/// name and type, so reporting them takes knowing which unit the statement defined.
+public struct OraclePLSQLUnit: Sendable, Equatable {
+    /// The type as `ALL_ERRORS.TYPE` spells it, such as `PACKAGE BODY`.
+    public let type: String
+
+    /// The schema the header names, or nil when the unit is created in the session's current schema.
+    public let owner: String?
+    public let name: String
+
+    public init(type: String, owner: String?, name: String) {
+        self.type = type
+        self.owner = owner
+        self.name = name
+    }
+
+    private static let modifiers: Set<String> = ["OR", "REPLACE", "EDITIONABLE", "NONEDITIONABLE"]
+    private static let unitTypes: Set<String> = ["PROCEDURE", "FUNCTION", "PACKAGE", "TRIGGER", "TYPE"]
+
+    /// The unit `sql` creates, or nil when it creates something else or nothing at all.
+    public static func definition(in sql: String) -> OraclePLSQLUnit? {
+        var reader = HeaderReader(sql)
+        guard reader.nextWord() == "CREATE" else { return nil }
+        var word = reader.nextWord()
+        while let modifier = word, modifiers.contains(modifier) {
+            word = reader.nextWord()
+        }
+        guard let kind = word, unitTypes.contains(kind) else { return nil }
+        var type = kind
+        if kind == "PACKAGE" || kind == "TYPE", reader.peekWord() == "BODY" {
+            _ = reader.nextWord()
+            type = "\(kind) BODY"
+        }
+        if reader.peekWord() == "IF" {
+            _ = reader.nextWord()
+            guard reader.nextWord() == "NOT", reader.nextWord() == "EXISTS" else { return nil }
+        }
+        guard let first = reader.nextIdentifier() else { return nil }
+        guard reader.consumePeriod() else {
+            return OraclePLSQLUnit(type: type, owner: nil, name: first)
+        }
+        guard let second = reader.nextIdentifier() else { return nil }
+        return OraclePLSQLUnit(type: type, owner: first, name: second)
+    }
+
+    /// Whether `sql` is an anonymous block, which opens with `DECLARE` or `BEGIN` after any `<<label>>`.
+    ///
+    /// The driver reports a row count of 1 for every block it runs, which is not a number of rows anything changed.
+    public static func isAnonymousBlock(_ sql: String) -> Bool {
+        var reader = HeaderReader(sql)
+        reader.skipLabels()
+        let word = reader.nextWord()
+        return word == "DECLARE" || word == "BEGIN"
+    }
+
+    /// The compilation errors Oracle recorded for this unit, most recent compile only, in the order it reported them.
+    ///
+    /// Warnings are left out: with `PLSQL_WARNINGS` enabled Oracle records them against units that compiled.
+    public var errorsQuery: String {
+        let owner = owner.map { "'\(OracleSchemaQueries.escapeLiteral($0))'" }
+            ?? "SYS_CONTEXT('USERENV', 'CURRENT_SCHEMA')"
+        return """
+            SELECT LINE, POSITION, TEXT FROM ALL_ERRORS \
+            WHERE OWNER = \(owner) \
+            AND NAME = '\(OracleSchemaQueries.escapeLiteral(name))' \
+            AND TYPE = '\(OracleSchemaQueries.escapeLiteral(type))' \
+            AND ATTRIBUTE = 'ERROR' \
+            ORDER BY SEQUENCE
+            """
+    }
+
+    /// What the editor shows for a unit that was stored but does not compile, one error per line.
+    public func compilationFailureMessage(errors: [OracleCompilationError]) -> String {
+        let header = String(
+            format: String(localized: "%1$@ %2$@ was created with compilation errors:"),
+            type, name
+        )
+        let lines = errors.map { error in
+            String(
+                format: String(localized: "Line %1$lld, column %2$lld: %3$@"),
+                Int64(error.line), Int64(error.position), error.text
+            )
+        }
+        return ([header] + lines).joined(separator: "\n")
+    }
+}
+
+public struct OracleCompilationError: Sendable, Equatable {
+    public let line: Int
+    public let position: Int
+    public let text: String
+
+    public init(line: Int, position: Int, text: String) {
+        self.line = line
+        self.position = position
+        self.text = text.trimmingCharacters(in: .whitespacesAndNewlines)
+    }
+
+    /// Reads one `ALL_ERRORS` row as ``OraclePLSQLUnit/errorsQuery`` selects it.
+    public init?(row: [OracleRawCell]) {
+        guard row.count >= 3,
+              let line = row[0].stringValue.flatMap(Int.init),
+              let position = row[1].stringValue.flatMap(Int.init),
+              let text = row[2].stringValue
+        else {
+            return nil
+        }
+        self.init(line: line, position: position, text: text)
+    }
+}
+
+/// Reads the words and identifiers at the head of a statement, past comments.
+private struct HeaderReader {
+    private let scalars: [Unicode.Scalar]
+    private var index = 0
+
+    init(_ sql: String) {
+        scalars = Array(sql.unicodeScalars)
+    }
+
+    mutating func nextWord() -> String? {
+        skipTrivia()
+        let start = index
+        while index < scalars.count, Self.isWordScalar(scalars[index]) {
+            index += 1
+        }
+        guard index > start else { return nil }
+        return String(String.UnicodeScalarView(scalars[start..<index])).uppercased()
+    }
+
+    func peekWord() -> String? {
+        var copy = self
+        return copy.nextWord()
+    }
+
+    /// A quoted identifier keeps its case; an unquoted one is stored uppercased, as Oracle stores it.
+    mutating func nextIdentifier() -> String? {
+        skipTrivia()
+        guard index < scalars.count else { return nil }
+        guard scalars[index] == "\"" else { return nextWord() }
+        var name = String.UnicodeScalarView()
+        index += 1
+        while index < scalars.count {
+            if scalars[index] == "\"" {
+                guard index + 1 < scalars.count, scalars[index + 1] == "\"" else {
+                    index += 1
+                    return String(name)
+                }
+                name.append("\"")
+                index += 2
+                continue
+            }
+            name.append(scalars[index])
+            index += 1
+        }
+        return nil
+    }
+
+    mutating func skipLabels() {
+        while true {
+            skipTrivia()
+            guard index + 1 < scalars.count, scalars[index] == "<", scalars[index + 1] == "<" else { return }
+            index += 2
+            while index + 1 < scalars.count, !(scalars[index] == ">" && scalars[index + 1] == ">") {
+                index += 1
+            }
+            index = min(index + 2, scalars.count)
+        }
+    }
+
+    mutating func consumePeriod() -> Bool {
+        skipTrivia()
+        guard index < scalars.count, scalars[index] == "." else { return false }
+        index += 1
+        return true
+    }
+
+    private mutating func skipTrivia() {
+        while index < scalars.count {
+            let scalar = scalars[index]
+            if scalar.properties.isWhitespace {
+                index += 1
+                continue
+            }
+            if scalar == "-", index + 1 < scalars.count, scalars[index + 1] == "-" {
+                while index < scalars.count, scalars[index] != "\n" {
+                    index += 1
+                }
+                continue
+            }
+            if scalar == "/", index + 1 < scalars.count, scalars[index + 1] == "*" {
+                index += 2
+                while index + 1 < scalars.count, !(scalars[index] == "*" && scalars[index + 1] == "/") {
+                    index += 1
+                }
+                index = min(index + 2, scalars.count)
+                continue
+            }
+            return
+        }
+    }
+
+    private static func isWordScalar(_ scalar: Unicode.Scalar) -> Bool {
+        scalar.properties.isAlphabetic || ("0"..."9").contains(scalar) || scalar == "_" || scalar == "$"
+            || scalar == "#"
+    }
+}

Packages/TableProOracle/Tests/TableProOracleCoreTests/OraclePLSQLUnitTests.swift

@@ -0,0 +1,82 @@
+@testable import TableProOracleCore
+import XCTest
+
+/// Oracle stores a unit that does not compile and reports success, so the plugin reads the unit's errors back by the
+/// owner, name and type the `CREATE` header names. Reading the wrong name reports a broken unit as fine.
+final class OraclePLSQLUnitTests: XCTestCase {
+    func testReadsTheUnitEachHeaderDefines() {
+        let cases: [(sql: String, unit: OraclePLSQLUnit)] = [
+            ("CREATE PROCEDURE p IS BEGIN NULL; END;", OraclePLSQLUnit(type: "PROCEDURE", owner: nil, name: "P")),
+            (
+                "create or replace editionable function hr.f return number is begin return 1; end;",
+                OraclePLSQLUnit(type: "FUNCTION", owner: "HR", name: "F")
+            ),
+            ("CREATE OR REPLACE PACKAGE BODY pkg AS END;", OraclePLSQLUnit(type: "PACKAGE BODY", owner: nil, name: "PKG")),
+            ("CREATE PACKAGE pkg AS END;", OraclePLSQLUnit(type: "PACKAGE", owner: nil, name: "PKG")),
+            ("CREATE TYPE BODY t AS END;", OraclePLSQLUnit(type: "TYPE BODY", owner: nil, name: "T")),
+            ("CREATE TYPE t AS OBJECT (a NUMBER);", OraclePLSQLUnit(type: "TYPE", owner: nil, name: "T")),
+            (
+                "-- note\nCREATE OR REPLACE TRIGGER \"App\".\"Audit\" BEFORE INSERT ON x FOR EACH ROW BEGIN NULL; END;",
+                OraclePLSQLUnit(type: "TRIGGER", owner: "App", name: "Audit")
+            ),
+            (
+                "CREATE PROCEDURE IF NOT EXISTS p IS BEGIN NULL; END;",
+                OraclePLSQLUnit(type: "PROCEDURE", owner: nil, name: "P")
+            ),
+            ("CREATE NONEDITIONABLE PROCEDURE x$y#z IS BEGIN NULL; END;", OraclePLSQLUnit(type: "PROCEDURE", owner: nil, name: "X$Y#Z")),
+        ]
+        for example in cases {
+            XCTAssertEqual(OraclePLSQLUnit.definition(in: example.sql), example.unit, example.sql)
+        }
+    }
+
+    func testStatementsThatDefineNoUnit() {
+        for sql in [
+            "CREATE TABLE t (a NUMBER)",
+            "CREATE OR REPLACE VIEW v AS SELECT 1 FROM dual",
+            "BEGIN NULL; END;",
+            "DROP PROCEDURE p",
+            "SELECT 'CREATE PROCEDURE p' FROM dual",
+        ] {
+            XCTAssertNil(OraclePLSQLUnit.definition(in: sql), sql)
+        }
+    }
+
+    func testErrorsQueryEscapesAndFallsBackToTheSessionSchema() {
+        let named = OraclePLSQLUnit(type: "PACKAGE BODY", owner: "O'NEIL", name: "IT'S")
+        XCTAssertTrue(named.errorsQuery.contains("OWNER = 'O''NEIL'"))
+        XCTAssertTrue(named.errorsQuery.contains("NAME = 'IT''S'"))
+        XCTAssertTrue(named.errorsQuery.contains("TYPE = 'PACKAGE BODY'"))
+        XCTAssertTrue(named.errorsQuery.contains("ATTRIBUTE = 'ERROR'"))
+
+        let unqualified = OraclePLSQLUnit(type: "PROCEDURE", owner: nil, name: "P")
+        XCTAssertTrue(unqualified.errorsQuery.contains("OWNER = SYS_CONTEXT('USERENV', 'CURRENT_SCHEMA')"))
+    }
+
+    func testFailureMessageListsEveryError() {
+        let unit = OraclePLSQLUnit(type: "PROCEDURE", owner: nil, name: "P")
+        let message = unit.compilationFailureMessage(errors: [
+            OracleCompilationError(line: 5, position: 3, text: "PLS-00103: Encountered the symbol \"end-of-file\"\n"),
+            OracleCompilationError(line: 7, position: 1, text: "PL/SQL: Statement ignored"),
+        ])
+        XCTAssertEqual(message, """
+            PROCEDURE P was created with compilation errors:
+            Line 5, column 3: PLS-00103: Encountered the symbol "end-of-file"
+            Line 7, column 1: PL/SQL: Statement ignored
+            """)
+    }
+
+    func testCompilationErrorReadsAnErrorsRow() {
+        let error = OracleCompilationError(row: [.string("5"), .string("3"), .string("PLS-00103: x")])
+        XCTAssertEqual(error, OracleCompilationError(line: 5, position: 3, text: "PLS-00103: x"))
+        XCTAssertNil(OracleCompilationError(row: [.null, .string("3"), .string("x")]))
+    }
+
+    func testAnonymousBlocksAreRecognisedPastLabelsAndComments() {
+        XCTAssertTrue(OraclePLSQLUnit.isAnonymousBlock("BEGIN NULL; END;"))
+        XCTAssertTrue(OraclePLSQLUnit.isAnonymousBlock("declare v number; begin null; end;"))
+        XCTAssertTrue(OraclePLSQLUnit.isAnonymousBlock("-- c\n<<outer>>\n<<inner>> BEGIN NULL; END;"))
+        XCTAssertFalse(OraclePLSQLUnit.isAnonymousBlock("SELECT 1 FROM dual"))
+        XCTAssertFalse(OraclePLSQLUnit.isAnonymousBlock("CALL p()"))
+    }
+}

Plugins/OracleDriverPlugin/OraclePlugin.swift

@@ -305,17 +305,33 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
         // Health monitor sends "SELECT 1" as a ping; Oracle requires FROM DUAL.
         let isBareSelectOne = query.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() == "select 1"
         var result = try await rawQuery(isBareSelectOne ? OracleSchemaQueries.ping : query)
+        try await reportCompilationErrors(of: query)
         let executionTime = Date().timeIntervalSince(startTime)
 
         // OracleNIO may not populate column metadata for empty result sets.
         if result.columns.isEmpty, result.rows.isEmpty,
            let recovered = try? await emptyResultColumns(for: query) {
             result = recovered
         }
+        if OraclePLSQLUnit.isAnonymousBlock(query) {
+            result = OracleRawResult(columns: result.columns, rows: result.rows, affectedRows: 0, isTruncated: false)
+        }
 
         return result.toPluginResult(executionTime: executionTime)
     }
 
+    /// Turns a `CREATE` that stored an INVALID unit into the failure it is.
+    ///
+    /// Oracle accepts the statement and flags the compile failure only as a warning, which oracle-nio drops, so the
+    /// unit's own errors are read back from `ALL_ERRORS`. A unit the header does not name, or one that compiled, adds
+    /// nothing.
+    func reportCompilationErrors(of query: String) async throws {
+        guard let unit = OraclePLSQLUnit.definition(in: query) else { return }
+        let errors = try await rawQuery(unit.errorsQuery).rows.compactMap(OracleCompilationError.init(row:))
+        guard !errors.isEmpty else { return }
+        throw OraclePluginError(core: .queryFailed(unit.compilationFailureMessage(errors: errors)))
+    }
+
     internal func rawQuery(_ query: String) async throws -> OracleRawResult {
         guard let core else { throw OraclePluginError(core: .notConnected) }
         do {
@@ -345,7 +361,9 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable {
     // MARK: - Streaming
 
     func executeBoundedQuery(query: String, rowCap: Int) async throws -> PluginQueryResult? {
-        try await boundedQueryFromStream(query: query, rowCap: rowCap)
+        let result = try await boundedQueryFromStream(query: query, rowCap: rowCap)
+        try await reportCompilationErrors(of: query)
+        return result
     }
 
     func streamRows(query: String) -> AsyncThrowingStream<PluginStreamElement, Error> {

Plugins/TableProPluginKit/SqlDialect.swift

@@ -4,6 +4,7 @@ public enum SqlDialect: String, Sendable, CaseIterable {
     case postgres
     case mysql
     case sqlite
+    case oracle
     case generic
 
     public static func from(databaseTypeId: String) -> SqlDialect {
@@ -14,6 +15,8 @@ public enum SqlDialect: String, Sendable, CaseIterable {
             return .mysql
         case "SQLite", "libSQL", "Turso", "DuckDB", "Cloudflare D1":
             return .sqlite
+        case "Oracle":
+            return .oracle
         default:
             return .generic
         }
@@ -38,4 +41,16 @@ public enum SqlDialect: String, Sendable, CaseIterable {
     public var supportsAdjacentStringConcatenation: Bool {
         self != .mysql
     }
+
+    /// Oracle's `q'[...]'` literal, whose body runs to the matching delimiter followed by a quote, so a lone `'`
+    /// inside it does not end it.
+    public var supportsAlternativeQuoting: Bool {
+        self == .oracle
+    }
+
+    /// SQL*Plus ends a statement at a line holding only `/`, which is how a PL/SQL unit is terminated in every script
+    /// written for SQL*Plus, SQLcl or SQL Developer. The line is a client command, never text the server accepts.
+    public var endsStatementsAtSlashLines: Bool {
+        self == .oracle
+    }
 }