fix(plugin-oracle): commit Oracle statements the way every other engine does

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Acknowledgements and a privacy policy link under **Settings > About** on iPhone and iPad.
 - Privacy manifest for the iOS app.
 - Oracle `DBMS_OUTPUT` lines shown with the result of the statement that printed them, and in a new **Output** result view.
+- Oracle transactions opened with `SET TRANSACTION`, `SAVEPOINT` or `LOCK TABLE`, held until `COMMIT` or `ROLLBACK`.
 
 ### Changed
 
@@ -44,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - 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.
+- Oracle statements run outside a transaction never committed, on Mac and on iPhone and iPad.
 - Oracle table and database metadata failing to load because its size query read `ALL_SEGMENTS`, a view Oracle does not have.
 - 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.

Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift

@@ -54,10 +54,15 @@ public final class OracleCoreConnection: @unchecked Sendable {
     private let unsupportedWarner = UnsupportedTypeWarner()
     private let nioLogger = Logging.Logger(label: "com.TablePro.oracle-nio")
 
+    /// Restoring a replaced session's settings writes nothing, so it neither commits nor joins a transaction.
+    private static let sessionSetupOptions = StatementOptions(autoCommit: false)
+
     private struct LockedState: Sendable {
         var isConnected = false
         var hasEverConnected = false
         var nioConnection: OracleNIO.OracleConnection?
+        var sessionID = 0
+        var transaction = OracleSessionTransaction()
         var queryTimeoutSeconds = 0
         var sessionSchema: String?
         var capturesServerOutput = false
@@ -130,6 +135,7 @@ public final class OracleCoreConnection: @unchecked Sendable {
 
             state.withLock { current in
                 current.nioConnection = connection
+                current.sessionID = connectionId
                 current.isConnected = true
                 current.hasEverConnected = true
             }
@@ -322,17 +328,75 @@ public final class OracleCoreConnection: @unchecked Sendable {
         let connection = try requireConnection()
         if let schema = state.withLock({ $0.sessionSchema }) {
             _ = try await withQueryDeadline { [self] in
-                try await collectRows(OracleSchemaQueries.setCurrentSchema(schema), on: connection)
+                try await collectRows(
+                    OracleSchemaQueries.setCurrentSchema(schema),
+                    options: Self.sessionSetupOptions,
+                    on: connection
+                )
             }
         }
         if state.withLock({ $0.capturesServerOutput }) {
             _ = try await withQueryDeadline { [self] in
-                try await collectRows(OracleServerOutput.enableStatement, on: connection)
+                try await collectRows(
+                    OracleServerOutput.enableStatement,
+                    options: Self.sessionSetupOptions,
+                    on: connection
+                )
             }
         }
         return connection
     }
 
+    // MARK: - Transactions
+
+    /// Whether a transaction is open on this session, from ``beginTransaction()`` or from a statement that opens one,
+    /// until a `COMMIT` or `ROLLBACK` ends it.
+    public var holdsTransaction: Bool {
+        state.withLock { $0.transaction.isOpen }
+    }
+
+    /// Holds every statement that follows in one transaction, until a `COMMIT` or `ROLLBACK` runs on the session.
+    ///
+    /// Oracle has no statement that opens a transaction the way `BEGIN` does elsewhere: the first write opens one. So
+    /// this sends nothing, and the statements after it simply stop committing as they run.
+    public func beginTransaction() {
+        state.withLock { $0.transaction.open() }
+    }
+
+    /// The options a statement in `role` runs with, and the connection it runs on. Read under the query gate and after
+    /// any reconnect, so the connection a transaction is bound to is the one the statement runs on.
+    private func admit(_ role: OracleTransactionRole) throws -> (options: StatementOptions, session: Int) {
+        try state.withLock { current in
+            let autoCommit = try current.transaction.admit(role, on: current.sessionID)
+            return (StatementOptions(autoCommit: autoCommit), current.sessionID)
+        }
+    }
+
+    private func recordSuccess(of role: OracleTransactionRole, on session: Int) {
+        state.withLock { $0.transaction.statementSucceeded(role, on: session) }
+    }
+
+    /// Called under the query gate, so the answer about the transaction comes from the connection the refused
+    /// `COMMIT` or `ROLLBACK` ran on.
+    private func recordFailure(of role: OracleTransactionRole) async {
+        guard role == .endsTransaction, holdsTransaction else { return }
+        let serverHoldsTransaction = await serverHoldsTransaction()
+        state.withLock { $0.transaction.statementFailed(role, serverHoldsTransaction: serverHoldsTransaction) }
+    }
+
+    private func serverHoldsTransaction() async -> Bool? {
+        guard let connection = state.withLock({ $0.isConnected ? $0.nioConnection : nil }) else { return nil }
+        let answer = try? await withQueryDeadline { [self] in
+            try await collectRows(
+                OracleSessionTransaction.serverTransactionQuery,
+                options: Self.sessionSetupOptions,
+                on: connection
+            )
+        }
+        guard let answer else { return nil }
+        return answer.rows.first?.first.map { $0 != .null } ?? false
+    }
+
     // MARK: - Server Output
 
     /// Turns `DBMS_OUTPUT` on for this session, and for every session a reconnect replaces it with, since the setting
@@ -444,31 +508,36 @@ public final class OracleCoreConnection: @unchecked Sendable {
     }
 
     public func executeQuery(_ query: String) async throws -> OracleRawResult {
+        let role = OracleTransactionRole(of: query)
         await queryGate.acquire()
 
         do {
             let connection = try await reconnectedConnection()
+            let admitted = try admit(role)
             let result = try await withQueryDeadline { [self] in
-                try await collectRows(query, on: connection)
+                try await collectRows(query, options: admitted.options, on: connection)
             }
+            recordSuccess(of: role, on: admitted.session)
             await queryGate.release()
             return result
         } catch {
             /// Classified before the gate is released, because releasing it resumes a queued caller that
             /// can redial and install a new connection. Marking the failure dead after that would tear
             /// down the connection the next query is already running on.
             let mapped = mapExecutionError(error)
+            await recordFailure(of: role)
             await queryGate.release()
             throw mapped
         }
     }
 
     private func collectRows(
         _ query: String,
+        options: StatementOptions,
         on connection: OracleNIO.OracleConnection
     ) async throws -> OracleRawResult {
         let statement = OracleStatement(stringLiteral: query)
-        let stream = try await connection.execute(statement, logger: nioLogger)
+        let stream = try await connection.execute(statement, options: options, logger: nioLogger)
 
         let columnNames = stream.columns.map(\.name)
         var columnTypeNames: [String] = []
@@ -521,29 +590,34 @@ public final class OracleCoreConnection: @unchecked Sendable {
         _ query: String,
         continuation: AsyncThrowingStream<OracleStreamElement, Error>.Continuation
     ) async throws {
+        let role = OracleTransactionRole(of: query)
         await queryGate.acquire()
 
         do {
             let connection = try await reconnectedConnection()
+            let admitted = try admit(role)
             try await withQueryDeadline { [self] in
-                try await streamRows(query, on: connection, continuation: continuation)
+                try await streamRows(query, options: admitted.options, on: connection, continuation: continuation)
             }
+            recordSuccess(of: role, on: admitted.session)
             await queryGate.release()
             continuation.finish()
         } catch {
             let mapped = mapExecutionError(error)
+            await recordFailure(of: role)
             await queryGate.release()
             throw mapped
         }
     }
 
     private func streamRows(
         _ query: String,
+        options: StatementOptions,
         on connection: OracleNIO.OracleConnection,
         continuation: AsyncThrowingStream<OracleStreamElement, Error>.Continuation
     ) async throws {
         let statement = OracleStatement(stringLiteral: query)
-        let stream = try await connection.execute(statement, logger: nioLogger)
+        let stream = try await connection.execute(statement, options: options, logger: nioLogger)
 
         let columnNames = stream.columns.map(\.name)
         var columnTypeNames: [String] = []

Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreError.swift

@@ -21,6 +21,7 @@ public enum OracleCoreError: LocalizedError, Sendable, Equatable {
     case protocolError
     case loginTimedOut
     case queryTimedOut
+    case transactionLost
     case authVerifierUnsupported(flag: String)
     case authVersionNotSupported
     case authConnectionDropped(phase: String?)
@@ -46,6 +47,8 @@ public enum OracleCoreError: LocalizedError, Sendable, Equatable {
             return String(localized: "Timed out during the Oracle login handshake. The server accepted the network connection but did not finish logging in.")
         case .queryTimedOut:
             return String(localized: "The query did not finish within the configured timeout, so the connection was reset. Run the query again.")
+        case .transactionLost:
+            return String(localized: "The connection was lost while a transaction was open. Check which of its changes were saved before running them again.")
         case .authVerifierUnsupported:
             return String(localized: "This account uses a password verifier the database driver does not support.")
         case .authVersionNotSupported:

Packages/TableProOracle/Sources/TableProOracleCore/OraclePLSQLUnit.swift

@@ -115,7 +115,7 @@ public struct OracleCompilationError: Sendable, Equatable {
 }
 
 /// Reads the words and identifiers at the head of a statement, past comments.
-private struct HeaderReader {
+struct HeaderReader {
     private let scalars: [Unicode.Scalar]
     private var index = 0
 

Packages/TableProOracle/Sources/TableProOracleCore/OracleSessionTransaction.swift

@@ -0,0 +1,81 @@
+import Foundation
+
+/// Whether the session has a transaction open, and which connection it lives on.
+///
+/// Oracle never commits on its own: every write joins a transaction that stays open until something commits it, and
+/// oracle-nio sends no commit unless a statement asks for one. TablePro presents every engine the same way, with each
+/// statement committed as it runs unless a transaction is open, so a statement run with none open carries the
+/// driver's commit flag and the server commits it in the same round trip.
+///
+/// A transaction opens with ``open()``, which is what the app's own writes call, or with a statement that opens one
+/// (``OracleTransactionRole/opensTransaction``). It ends with a `COMMIT` or `ROLLBACK`, whoever sends it.
+///
+/// The transaction belongs to the connection its first statement ran on. A connection that closes takes the
+/// transaction with it, so a statement that finds the transaction's connection replaced fails with
+/// ``OracleCoreError/transactionLost`` rather than carrying on in a new session that holds none of the earlier work.
+struct OracleSessionTransaction: Equatable, Sendable {
+    private(set) var isOpen = false
+    private var session: Int?
+
+    mutating func open() {
+        isOpen = true
+    }
+
+    /// Whether a statement in `role` runs with the commit flag on the connection `session`.
+    ///
+    /// Inside a transaction nothing commits on its own, and the transaction is bound to `session` if no statement has
+    /// bound it yet. A query takes no part: it writes nothing, and it may run on any connection.
+    mutating func admit(_ role: OracleTransactionRole, on session: Int) throws -> Bool {
+        switch role {
+        case .query:
+            return false
+        case .opensTransaction, .endsTransaction:
+            guard isOpen else { return false }
+        case .other:
+            guard isOpen else { return true }
+        }
+        try bind(to: session)
+        return false
+    }
+
+    /// A statement that opens a transaction opens it here only once Oracle has accepted it, and a `COMMIT` or
+    /// `ROLLBACK` ends it.
+    mutating func statementSucceeded(_ role: OracleTransactionRole, on session: Int) {
+        switch role {
+        case .opensTransaction:
+            isOpen = true
+            self.session = session
+        case .endsTransaction:
+            close()
+        case .query, .other:
+            break
+        }
+    }
+
+    /// A `COMMIT` or `ROLLBACK` the server refused ends the transaction only when the server no longer holds one: a
+    /// malformed one leaves it open, and a commit a deferred constraint rolled back (ORA-02091) ends it.
+    ///
+    /// `serverHoldsTransaction` is nil when there was no live connection to ask. The transaction is then left as it
+    /// is, and the next write finds out whether its connection survived.
+    mutating func statementFailed(_ role: OracleTransactionRole, serverHoldsTransaction: Bool?) {
+        guard role == .endsTransaction, serverHoldsTransaction == false else { return }
+        close()
+    }
+
+    /// What the server answers for the session's own transaction: its id while one is open, NULL otherwise. Named
+    /// with its owner, because a bare `DBMS_TRANSACTION` resolves to an object of that name in the current schema first.
+    static let serverTransactionQuery = "SELECT SYS.DBMS_TRANSACTION.LOCAL_TRANSACTION_ID FROM SYS.DUAL"
+
+    private mutating func bind(to session: Int) throws {
+        if let bound = self.session, bound != session {
+            close()
+            throw OracleCoreError.transactionLost
+        }
+        self.session = session
+    }
+
+    private mutating func close() {
+        isOpen = false
+        session = nil
+    }
+}

Packages/TableProOracle/Sources/TableProOracleCore/OracleTransactionRole.swift

@@ -0,0 +1,52 @@
+import Foundation
+
+/// What a statement does to the session's transaction, read from its first words.
+enum OracleTransactionRole: Equatable, Sendable {
+    /// `SELECT` or `WITH`. A query writes nothing, so it never needs a commit, and a commit sent with a
+    /// `SELECT ... FOR UPDATE` ends the transaction its cursor belongs to: measured on Oracle 23ai, fetching past the
+    /// first round trip then fails with ORA-01002.
+    case query
+
+    /// `SET TRANSACTION`, `SAVEPOINT` or `LOCK TABLE`. Each one only means something inside a transaction, and Oracle
+    /// opens one for it (measured: `DBMS_TRANSACTION.LOCAL_TRANSACTION_ID` is set after a `SAVEPOINT` alone), so it
+    /// opens one here rather than being committed away the moment it runs.
+    case opensTransaction
+
+    /// `COMMIT`, or a `ROLLBACK` that is not to a savepoint.
+    case endsTransaction
+
+    /// Everything else: DML, DDL, PL/SQL, `CALL`, `ROLLBACK TO`, and the `FORCE` forms of `COMMIT` and `ROLLBACK`,
+    /// which settle an in-doubt distributed transaction rather than the session's own.
+    case other
+
+    /// How much of a statement is read for its first words. A dump can hold a statement millions of characters long,
+    /// and every statement the driver runs is read here.
+    private static let headerScanLimit = 4_096
+
+    init(of sql: String) {
+        var reader = HeaderReader(String(String.UnicodeScalarView(sql.unicodeScalars.prefix(Self.headerScanLimit))))
+        switch reader.nextWord() {
+        case "SELECT", "WITH":
+            self = .query
+        case "SAVEPOINT":
+            self = .opensTransaction
+        case "SET":
+            self = reader.nextWord() == "TRANSACTION" ? .opensTransaction : .other
+        case "LOCK":
+            self = reader.nextWord() == "TABLE" ? .opensTransaction : .other
+        case "COMMIT":
+            self = Self.skippingWork(&reader) == "FORCE" ? .other : .endsTransaction
+        case "ROLLBACK":
+            let word = Self.skippingWork(&reader)
+            self = word == "TO" || word == "FORCE" ? .other : .endsTransaction
+        default:
+            self = .other
+        }
+    }
+
+    private static func skippingWork(_ reader: inout HeaderReader) -> String? {
+        let word = reader.nextWord()
+        guard word == "WORK" else { return word }
+        return reader.nextWord()
+    }
+}