From 1c235337babd0c3ba14be2b712dba87991c4ae9f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 18:52:37 +0700 Subject: [PATCH 1/4] feat(postgresql): show server notices with each statement's result --- .../OracleCoreConnection.swift | 16 +- .../OracleServerOutputPending.swift | 12 ++ .../OracleServerOutputPendingTests.swift | 39 +++++ .../MSSQLDriverPlugin/FreeTDSConnection.swift | 13 +- .../LibPQDriverCore.swift | 9 ++ .../LibPQNoticeOutput.swift | 62 ++++++++ .../LibPQPluginConnection.swift | 18 ++- .../LibPQServerMessageSink.swift | 28 +++- .../LibPQNoticeOutputTests.swift | 130 +++++++++++++++ .../Coordinators/PaginationCoordinator.swift | 26 ++- .../QueryExecutionCoordinator+Batches.swift | 26 ++- .../QueryExecutionCoordinator+Helpers.swift | 9 +- ...QueryExecutionCoordinator+Parameters.swift | 12 +- .../Execution/BatchStatementRun.swift | 27 +++- .../Services/Query/ServerOutputCapture.swift | 16 +- .../Query/ExplainResultSetFactory.swift | 5 +- .../TablePro/Models/Query/QueryTabState.swift | 6 + .../Main/Child/MainEditorContentView.swift | 1 + .../MainContentCoordinator+Explain.swift | 22 ++- .../Views/QueryPlan/QueryPlanResultView.swift | 89 +++++++---- .../Execution/BatchStatementRunTests.swift | 74 ++++++++- .../Query/ServerOutputCaptureTests.swift | 93 ++++++++++- .../Views/Main/FetchAllQueryTaskTests.swift | 49 ++++++ .../QueryPlanPresentationTests.swift | 42 ++++- .../probes/check-postgresql-notices.sh | 111 +++++++++++++ core/crates/core/src/result_event.rs | 5 + .../drivers/postgres/src/editor/actor.rs | 6 +- .../postgres/src/editor/editor_session.rs | 3 +- .../crates/drivers/postgres/src/editor/mod.rs | 1 + .../postgres/src/editor/notice_queue.rs | 150 ++++++++++++++++++ .../crates/drivers/postgres/src/editor/run.rs | 74 ++++++--- .../postgres/src/editor/session_core.rs | 66 ++++++-- .../tests/integration/editor_session.rs | 108 +++++++++++++ core/crates/ffi/src/run/run_event.rs | 5 + core/crates/ffi/src/run/run_sink.rs | 41 +++++ core/crates/mcp/src/collected_results.rs | 2 + linux/crates/app/src/ui/editor/run_log.rs | 49 ++++++ linux/docs/adding-drivers.md | 1 + linux/docs/drivers/postgresql.md | 2 +- windows/docs/decisions/0004-app-shell.md | 5 +- windows/src/TablePro.App.Model/RunMessages.cs | 43 +++++ windows/src/TablePro.App/MainWindow.xaml.cs | 10 ++ .../TablePro.App/Strings/en-US/Resources.resw | 15 ++ windows/src/TablePro.App/Views/ITabContent.cs | 4 + .../TablePro.App/Views/QueryTabView.xaml.cs | 18 ++- .../src/TablePro.App/Views/ResultsPane.xaml | 67 ++++++-- .../TablePro.App/Views/ResultsPane.xaml.cs | 60 +++++++ .../TablePro.App/Views/TableTabView.xaml.cs | 2 + .../Generated/tablepro_ffi.cs | 21 ++- windows/src/TablePro.Interop/Mapping.cs | 1 + windows/src/TablePro.Interop/QueryEvent.cs | 3 + .../RunMessagesTests.cs | 64 ++++++++ .../tests/TablePro.App.UITests/NoticeTests.cs | 60 +++++++ .../TablePro.Interop.Tests/NoticeTests.cs | 67 ++++++++ 54 files changed, 1741 insertions(+), 147 deletions(-) create mode 100644 apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleServerOutputPending.swift create mode 100644 apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleServerOutputPendingTests.swift create mode 100644 apple/Plugins/PostgreSQLDriverPlugin/LibPQNoticeOutput.swift create mode 100644 apple/Plugins/PostgreSQLDriverTests/LibPQNoticeOutputTests.swift create mode 100755 apple/scripts/probes/check-postgresql-notices.sh create mode 100644 core/crates/drivers/postgres/src/editor/notice_queue.rs create mode 100644 windows/src/TablePro.App.Model/RunMessages.cs create mode 100644 windows/tests/TablePro.App.Model.Tests/RunMessagesTests.cs create mode 100644 windows/tests/TablePro.App.UITests/NoticeTests.cs create mode 100644 windows/tests/TablePro.Interop.Tests/NoticeTests.cs diff --git a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift index 233ae317d6..fa25fee925 100644 --- a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift +++ b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleCoreConnection.swift @@ -105,6 +105,7 @@ public final class OracleCoreConnection: @unchecked Sendable { var queryTimeoutSeconds = 0 var sessionSchema: String? var capturesServerOutput = false + var pendingServerOutput = OracleServerOutputPending() var close = OracleCloseRecord() var serverRelease: OracleServerRelease? var loginWarning: String? @@ -635,6 +636,7 @@ public final class OracleCoreConnection: @unchecked Sendable { await queryGate.acquire() do { let connection = try requireConnection() + noteStatement() let rows = try await withQueryDeadline { [self] in try await runDirect(sql, binds: binds, autoCommit: autoCommit, readsRows: readsRows, on: connection) } @@ -696,7 +698,11 @@ public final class OracleCoreConnection: @unchecked Sendable { guard state.withLock({ $0.capturesServerOutput }), maxLines > 0 else { return .empty } await queryGate.acquire() - guard let connection = state.withLock({ $0.isConnected ? $0.nioConnection : nil }) else { + let connection = state.withLock { current -> OracleNIO.OracleConnection? in + guard current.isConnected, current.pendingServerOutput.needsRead else { return nil } + return current.nioConnection + } + guard let connection else { await queryGate.release() return .empty } @@ -704,6 +710,7 @@ public final class OracleCoreConnection: @unchecked Sendable { let output = try await withQueryDeadline { [self] in try await readServerOutput(on: connection, maxLines: maxLines) } + state.withLock { $0.pendingServerOutput.noteRead() } await queryGate.release() return output } catch { @@ -713,6 +720,10 @@ public final class OracleCoreConnection: @unchecked Sendable { } } + private func noteStatement() { + state.withLock { $0.pendingServerOutput.noteStatement() } + } + private func readServerOutput( on connection: OracleNIO.OracleConnection, maxLines: Int @@ -827,6 +838,7 @@ public final class OracleCoreConnection: @unchecked Sendable { do { let connection = try await reconnectedConnection() + noteStatement() let admitted = try admit(role) let result = try await withQueryDeadline { [self] in let zones = await sessionZones(on: connection) @@ -859,6 +871,7 @@ public final class OracleCoreConnection: @unchecked Sendable { let admitted: (options: StatementOptions, session: Int) do { connection = try await reconnectedConnection() + noteStatement() admitted = try admit(role) try await withQueryDeadline { [self] in try await discardRows(OracleStatement(unsafeSQL: statement), options: admitted.options, on: connection) @@ -983,6 +996,7 @@ public final class OracleCoreConnection: @unchecked Sendable { do { let connection = try await reconnectedConnection() + noteStatement() let admitted = try admit(role) try await withQueryDeadline { [self] in let zones = await sessionZones(on: connection) diff --git a/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleServerOutputPending.swift b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleServerOutputPending.swift new file mode 100644 index 0000000000..48f13ec19c --- /dev/null +++ b/apple/Packages/TableProOracle/Sources/TableProOracleCore/OracleServerOutputPending.swift @@ -0,0 +1,12 @@ +// A reconnect never clears this: a stale true costs one round trip, a stale false loses a statement's DBMS_OUTPUT lines. +internal struct OracleServerOutputPending: Sendable, Equatable { + private(set) var needsRead = false + + mutating func noteStatement() { + needsRead = true + } + + mutating func noteRead() { + needsRead = false + } +} diff --git a/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleServerOutputPendingTests.swift b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleServerOutputPendingTests.swift new file mode 100644 index 0000000000..fe51999fb3 --- /dev/null +++ b/apple/Packages/TableProOracle/Tests/TableProOracleCoreTests/OracleServerOutputPendingTests.swift @@ -0,0 +1,39 @@ +@testable import TableProOracleCore +import Testing + +// The host reads DBMS_OUTPUT before and after every editor statement, and on Oracle each read is a round trip. +struct OracleServerOutputPendingTests { + @Test func testANewSessionHasNothingToRead() { + #expect(!OracleServerOutputPending().needsRead) + } + + @Test func testAStatementLeavesOutputToRead() { + var pending = OracleServerOutputPending() + pending.noteStatement() + #expect(pending.needsRead) + } + + @Test func testOneReadTakesWhatEveryStatementBeforeItLeft() { + var pending = OracleServerOutputPending() + pending.noteStatement() + pending.noteStatement() + pending.noteRead() + #expect(!pending.needsRead) + } + + @Test func testAReadBeforeTheNextStatementFindsNothingNew() { + var pending = OracleServerOutputPending() + pending.noteStatement() + pending.noteRead() + pending.noteRead() + #expect(!pending.needsRead) + } + + @Test func testAStatementAfterAReadNeedsAnotherRead() { + var pending = OracleServerOutputPending() + pending.noteStatement() + pending.noteRead() + pending.noteStatement() + #expect(pending.needsRead) + } +} diff --git a/apple/Plugins/MSSQLDriverPlugin/FreeTDSConnection.swift b/apple/Plugins/MSSQLDriverPlugin/FreeTDSConnection.swift index 49e07e136f..c2a30356ce 100644 --- a/apple/Plugins/MSSQLDriverPlugin/FreeTDSConnection.swift +++ b/apple/Plugins/MSSQLDriverPlugin/FreeTDSConnection.swift @@ -101,6 +101,14 @@ nonisolated private func freetdsDiagnostics(for dbproc: UnsafeMutablePointer +) { + let key = freetdsConnectionKey(dbproc) + freetdsErrors.withLock { $0.perConnection[key] = diagnostics } +} + /// The output of the connection's latest request, handed over once. nonisolated private func freetdsTakeOutput( for dbproc: UnsafeMutablePointer @@ -585,7 +593,9 @@ nonisolated final class FreeTDSConnection: @unchecked Sendable { answer = .failure(error) } - guard dbproc != nil else { return try answer.get() } + guard let proc = dbproc else { return try answer.get() } + // The reset clears the session's messages, and the statement's PRINT output is what the next read takes. + let statementDiagnostics = freetdsDiagnostics(for: proc) // Numbered after the statement, so a Stop that ended it does not skip the reset. let resetCall = requestGate.enqueue() do { @@ -597,6 +607,7 @@ nonisolated final class FreeTDSConnection: @unchecked Sendable { disconnect() throw MSSQLCoreError.queryFailed(MSSQLShowplan.resetFailedMessage) } + freetdsRestoreDiagnostics(statementDiagnostics, for: proc) return try answer.get() } diff --git a/apple/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift b/apple/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift index 5ca2080fbb..4fa8c7c42f 100644 --- a/apple/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift +++ b/apple/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift @@ -320,6 +320,11 @@ final class LibPQDriverCore: @unchecked Sendable { libpqConnection?.observeServerNotices(observer) } + func fetchServerOutput() -> PluginServerOutput { + guard let connection = libpqConnection else { return .none } + return connection.takeServerOutput() + } + func nextNotifications(waitMicroseconds: Int64) async throws -> [LibPQNotification] { guard let connection = libpqConnection else { throw LibPQPluginError.notConnected } return try await connection.nextNotifications(waitMicroseconds: waitMicroseconds) @@ -548,6 +553,10 @@ extension LibPQBackedDriver { await core.sessionTransactionState() } + func fetchServerOutput() async throws -> PluginServerOutput { + core.fetchServerOutput() + } + func switchSchema(to schema: String) async throws { try await core.applySchema(schema) } diff --git a/apple/Plugins/PostgreSQLDriverPlugin/LibPQNoticeOutput.swift b/apple/Plugins/PostgreSQLDriverPlugin/LibPQNoticeOutput.swift new file mode 100644 index 0000000000..7940aae3be --- /dev/null +++ b/apple/Plugins/PostgreSQLDriverPlugin/LibPQNoticeOutput.swift @@ -0,0 +1,62 @@ +// +// LibPQNoticeOutput.swift +// PostgreSQLDriverPlugin +// + +import Foundation +import TableProPluginKit + +struct LibPQNoticeOutput: Sendable { + static let lineLimit = 10_000 + static let lineByteLimit = 32_767 + + private var lines: [String] = [] + private var isTruncated = false + + // libpq's own notices carry no SQLSTATE, and a session-ending message is reported as the lost connection instead. + static func isOutput(severity: String?, sqlState: String?) -> Bool { + guard let sqlState, !sqlState.isEmpty else { return false } + return !LibPQServerMessage.endsSession(severity: severity, sqlState: sqlState) + } + + // Scalars, not Characters: "\r\n" is one Character, so a message ending in "\r" would keep libpq's newline. + mutating func record(_ text: String) { + guard hasRoom() else { return } + var body = Substring(text).unicodeScalars + if body.last == "\n" { body.removeLast() } + guard !body.isEmpty else { return } + // One line at a time: a notice can hold millions of newlines, and only the first ones fit. + var start = body.startIndex + while hasRoom() { + let end = body[start...].firstIndex(of: "\n") ?? body.endIndex + lines.append(clipped(body[start.. PluginServerOutput { + let output = PluginServerOutput(lines: lines, isTruncated: isTruncated) + self = Self() + return output + } + + private mutating func hasRoom() -> Bool { + guard lines.count < Self.lineLimit else { + isTruncated = true + return false + } + return true + } + + private mutating func clipped(_ line: Substring.UnicodeScalarView) -> String { + let bytes = Substring(line).utf8 + guard bytes.count > Self.lineByteLimit else { return String(Substring(line)) } + isTruncated = true + var end = bytes.index(bytes.startIndex, offsetBy: Self.lineByteLimit) + while UTF8.isContinuation(bytes[end]) { + bytes.formIndex(before: &end) + } + return String(Substring(line[.. PluginServerOutput { + guard let sink = stateLock.withLock({ serverMessages?.takeUnretainedValue() }) else { return .none } + return sink.takeOutput() + } + + // Notices from a result an earlier statement left unread would otherwise show as this statement's output. + private func finishAbandonedResults(_ conn: OpaquePointer, cancellingOutput: Bool) { + guard let sink = stateLock.withLock({ serverMessages?.takeUnretainedValue() }) else { + _ = finishPendingResults(conn, cancellingOutput: cancellingOutput) + return + } + sink.discardingOutput { _ = finishPendingResults(conn, cancellingOutput: cancellingOutput) } + } + // A debug session runs with no statement timeout, so a call blocked on a server that stopped answering would // otherwise keep the socket, and the PQfinish queued behind it, for as long as TCP keeps the connection open. func interruptBlockedCallOnDisconnect() { diff --git a/apple/Plugins/PostgreSQLDriverPlugin/LibPQServerMessageSink.swift b/apple/Plugins/PostgreSQLDriverPlugin/LibPQServerMessageSink.swift index be78bb57fa..4b3fabaede 100644 --- a/apple/Plugins/PostgreSQLDriverPlugin/LibPQServerMessageSink.swift +++ b/apple/Plugins/PostgreSQLDriverPlugin/LibPQServerMessageSink.swift @@ -5,6 +5,7 @@ import CLibPQ import Foundation +import TableProPluginKit struct LibPQServerNotice: Sendable { let message: String @@ -19,6 +20,8 @@ final class LibPQServerMessageSink: @unchecked Sendable { private var sessionEnding: LibPQPluginError? private var forwardedReceiver: PQnoticeReceiver? private var noticeObserver: (@Sendable (LibPQServerNotice) -> Void)? + private var output = LibPQNoticeOutput() + private var discardsOutput = false static func install(on connection: OpaquePointer) -> Unmanaged { let retained = Unmanaged.passRetained(LibPQServerMessageSink()) @@ -33,10 +36,7 @@ final class LibPQServerMessageSink: @unchecked Sendable { lock.withLock { sessionEnding } } - /// Only ever called for a connection libpq still reports as `CONNECTION_OK`. A server before - /// 9.6 sends no non-localized severity, so a `RAISE WARNING ... ERRCODE '08006'` on a healthy - /// session reaches `receive` looking exactly like the real thing; a session that ended cannot - /// be healthy, so the healthy case is the one that must not keep the message. + // Before 9.6 the server sends no V field, so a healthy session's `RAISE WARNING ... ERRCODE '08006'` looks session-ending. func clearIfHealthy() { lock.withLock { sessionEnding = nil } } @@ -47,18 +47,32 @@ final class LibPQServerMessageSink: @unchecked Sendable { lock.withLock { noticeObserver = observer } } + func takeOutput() -> PluginServerOutput { + lock.withLock { output.take() } + } + + func discardingOutput(_ body: () throws -> Value) rethrows -> Value { + lock.withLock { discardsOutput = true } + defer { lock.withLock { discardsOutput = false } } + return try body() + } + fileprivate func receive(_ result: OpaquePointer) { - let message = PQresultErrorMessage(result).map { - String(cString: $0).trimmingCharacters(in: .whitespacesAndNewlines) - } ?? "" + let text = PQresultErrorMessage(result).map { String(cString: $0) } ?? "" + let message = text.trimmingCharacters(in: .whitespacesAndNewlines) let received = LibPQPluginError(message: message) { field in PQresultErrorField(result, field).map { String(cString: $0) } } let severity = PQresultErrorField(result, Self.severityField).map { String(cString: $0) } + let isOutput = LibPQNoticeOutput.isOutput(severity: severity, sqlState: received.sqlState) let (forward, observer) = lock.withLock { () -> (PQnoticeReceiver?, (@Sendable (LibPQServerNotice) -> Void)?) in if LibPQServerMessage.endsSession(severity: severity, sqlState: received.sqlState) { sessionEnding = received } + // The debugger shows what its observer gets, and nothing reads output from its target session. + if isOutput, noticeObserver == nil, !discardsOutput { + output.record(text) + } return (forwardedReceiver, noticeObserver) } if let observer { diff --git a/apple/Plugins/PostgreSQLDriverTests/LibPQNoticeOutputTests.swift b/apple/Plugins/PostgreSQLDriverTests/LibPQNoticeOutputTests.swift new file mode 100644 index 0000000000..43d362e6a1 --- /dev/null +++ b/apple/Plugins/PostgreSQLDriverTests/LibPQNoticeOutputTests.swift @@ -0,0 +1,130 @@ +// +// LibPQNoticeOutputTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +struct LibPQNoticeOutputTests { + private static let prefix = "NOTICE: " + + @Test("Each line of a notice, DETAIL and HINT included, is its own output line") + func splitsOnNewlines() { + var output = LibPQNoticeOutput() + output.record("NOTICE: with extras\nDETAIL: the detail\nHINT: the hint\n") + output.record("NOTICE: step 1 done\n") + #expect(output.take() == PluginServerOutput( + lines: ["NOTICE: with extras", "DETAIL: the detail", "HINT: the hint", "NOTICE: step 1 done"], + isTruncated: false + )) + } + + @Test("Only libpq's one trailing newline is dropped; the message's own whitespace stays") + func trailingNewlineOnly() { + var output = LibPQNoticeOutput() + output.record("NOTICE: padded \n") + output.record("NOTICE: ends blank\n\n") + output.record(" NOTICE: no newline") + #expect(output.take().lines == ["NOTICE: padded ", "NOTICE: ends blank", "", " NOTICE: no newline"]) + } + + @Test("A carriage return stays on its line, and libpq's newline after it is still dropped") + func carriageReturnStays() { + var output = LibPQNoticeOutput() + output.record("NOTICE: first\r\nsecond\r\n") + #expect(output.take().lines == ["NOTICE: first\r", "second\r"]) + } + + @Test("Past 10,000 lines nothing more is kept and the output reads as cut short") + func lineCap() { + var output = LibPQNoticeOutput() + for index in 1.. let plan: BatchTransactionPlan @@ -286,21 +292,25 @@ extension QueryExecutionCoordinator { var printed = BatchPrintedOutput() for _ in 0.. + var outcome: BatchStatementOutcome let plan: BatchTransactionPlan let sessionState: PluginSessionTransactionState let startState: PluginSessionTransactionState @@ -510,6 +510,7 @@ extension QueryExecutionCoordinator { lease: DriverLeaseOwner ) async -> MultiStatementRun { let failureOutput = ServerOutputBox() + let commitOutput = ServerOutputBox() let databaseType = parent.connection.type do { var run = try await DatabaseManager.shared.withScopedDriver( @@ -535,7 +536,8 @@ extension QueryExecutionCoordinator { for: databaseType, session: driver.sessionLexicalState ).execution ) - } + }, + commitOutput: commitOutput ) { statement in try await ServerOutputCapture.running(on: driver, failureOutput: failureOutput) { try await self.executeStatement( @@ -567,7 +569,11 @@ extension QueryExecutionCoordinator { endState: sessionState ) } - run.failureOutput = failureOutput.output + if run.outcome.isCompleted { + run.outcome = run.outcome.appendingCommitOutput(commitOutput.output) + } else { + run.failureOutput = failureOutput.output.appending(commitOutput.output) + } return run } catch { if DatabaseCancellationDiagnosis.isCancellation(error) || Task.isCancelled { diff --git a/apple/TablePro/Core/Services/Execution/BatchStatementRun.swift b/apple/TablePro/Core/Services/Execution/BatchStatementRun.swift index c9ab6e191e..3f3c01bb6d 100644 --- a/apple/TablePro/Core/Services/Execution/BatchStatementRun.swift +++ b/apple/TablePro/Core/Services/Execution/BatchStatementRun.swift @@ -46,6 +46,15 @@ internal extension BatchStatementOutcome { } } +internal extension BatchStatementOutcome where Output: CarriesServerOutput { + // The app's own COMMIT runs after the last statement, so what it printed (a deferred trigger's notices) shows there. + func appendingCommitOutput(_ output: PluginServerOutput) -> Self { + guard !output.isEmpty, case .completed(var results) = self, let last = results.indices.last else { return self } + results[last].serverOutput = results[last].serverOutput.appending(output) + return .completed(results: results) + } +} + /// The driver calls one multi-statement run makes, in order, for one ``BatchTransactionPlan``. /// /// Extracted from the coordinator so the order is testable without a window, a tab or a server: @@ -68,6 +77,7 @@ internal enum BatchStatementRun { isCommitPoint: (Statement) -> Bool, serverError: (Output) -> String? = { _ in nil }, replanned: @escaping @MainActor @Sendable (_ ranCount: Int) async -> [Statement]? = { _ in nil }, + commitOutput: ServerOutputBox = ServerOutputBox(), execute: @escaping @MainActor @Sendable (Statement) async throws -> Output ) async -> BatchStatementOutcome { let opensTransaction = plan.opensTransaction && driver.supportsTransactions @@ -131,7 +141,9 @@ internal enum BatchStatementRun { } guard opensTransaction else { return .completed(results: results) } - return await commit(results: results, driver: driver, connectionId: connectionId, gate: gate, plan: plan) + return await commit( + results: results, driver: driver, connectionId: connectionId, gate: gate, plan: plan, output: commitOutput + ) } /// A statement the script wrote itself is ordinary work unless it is the script's own commit, @@ -174,7 +186,8 @@ internal enum BatchStatementRun { driver: DatabaseDriver, connectionId: UUID, gate: BatchClaimGate, - plan: BatchTransactionPlan + plan: BatchTransactionPlan, + output: ServerOutputBox ) async -> BatchStatementOutcome { let committed = await BatchCommitPoint.run( driver: driver, @@ -188,10 +201,16 @@ internal enum BatchStatementRun { await rollback(driver: driver, connectionId: connectionId, plan: plan, opensTransaction: true) return .cancelled(results: plan.keepsExecutedStatements ? results : []) case .committed: + output.store(await ServerOutputCapture.drain(driver)) return .completed(results: results) - case .failed(let failure) where failure.outcomeIsUnknown: - return .failed(results: results, failure: .commitOutcomeUnknown, errorDescription: failure.errorDescription) case .failed(let failure): + // Read before the rollback: PostgreSQL answers a ROLLBACK after a failed COMMIT with a warning of its own. + output.store(await ServerOutputCapture.drain(driver)) + guard !failure.outcomeIsUnknown else { + return .failed( + results: results, failure: .commitOutcomeUnknown, errorDescription: failure.errorDescription + ) + } await rollback(driver: driver, connectionId: connectionId, plan: plan, opensTransaction: true) return .failed(results: results, failure: .commit, errorDescription: failure.errorDescription) } diff --git a/apple/TablePro/Core/Services/Query/ServerOutputCapture.swift b/apple/TablePro/Core/Services/Query/ServerOutputCapture.swift index 4dfef5642f..0d81adbdc9 100644 --- a/apple/TablePro/Core/Services/Query/ServerOutputCapture.swift +++ b/apple/TablePro/Core/Services/Query/ServerOutputCapture.swift @@ -17,9 +17,10 @@ extension QueryResult: CarriesServerOutput {} /// Reads what a statement the editor ran printed on the server, on the session that ran it. /// -/// The read happens after every statement, failed ones included, because the server hands the lines to whoever asks -/// next: output a failed statement left unread would be reported under the statement after it. A read that fails is -/// logged and reported as no output, since the statement it follows has already run. +/// The session hands its lines to whoever asks next, so they are read before the statement and discarded: what a +/// grid save, the toolbar Commit, an MCP or AI statement or a table read printed is never reported under this one. +/// The read after runs for a failed statement too, for the failure to report. A read that fails is logged and +/// reported as no output. enum ServerOutputCapture { private static let logger = Logger(subsystem: "com.TablePro", category: "ServerOutputCapture") @@ -28,6 +29,9 @@ enum ServerOutputCapture { failureOutput: ServerOutputBox, _ statement: () async throws -> Value ) async throws -> Value { + _ = await drain(driver) + // A Stop that landed during the read must not let the statement go out after it. + try Task.checkCancellation() do { var value = try await statement() value.serverOutput = await drain(driver) @@ -76,6 +80,12 @@ enum ServerOutputCapture { } } +extension PluginServerOutput { + func appending(_ later: PluginServerOutput) -> PluginServerOutput { + PluginServerOutput(lines: lines + later.lines, isTruncated: isTruncated || later.isTruncated) + } +} + /// Carries a failed statement's output from the driver's session to the main-actor code that reports the failure. final class ServerOutputBox: Sendable { private let state = OSAllocatedUnfairLock(initialState: PluginServerOutput.none) diff --git a/apple/TablePro/Models/Query/ExplainResultSetFactory.swift b/apple/TablePro/Models/Query/ExplainResultSetFactory.swift index a1dea5cc88..2cfd9b93c9 100644 --- a/apple/TablePro/Models/Query/ExplainResultSetFactory.swift +++ b/apple/TablePro/Models/Query/ExplainResultSetFactory.swift @@ -7,6 +7,7 @@ // import Foundation +import TableProPluginKit @MainActor enum ExplainResultSetFactory { @@ -16,7 +17,8 @@ enum ExplainResultSetFactory { sql: String?, executionTime: TimeInterval?, anchor: StatementAnchor? = nil, - planContext: QueryPlanContext? = nil + planContext: QueryPlanContext? = nil, + serverOutput: PluginServerOutput = .none ) -> ResultSet { let resultSet = ResultSet(label: String(localized: "Plan")) resultSet.explainRawText = rawText @@ -25,6 +27,7 @@ enum ExplainResultSetFactory { resultSet.executionTime = executionTime resultSet.statementAnchor = anchor resultSet.explainPlanContext = planContext + resultSet.serverOutput = serverOutput return resultSet } } diff --git a/apple/TablePro/Models/Query/QueryTabState.swift b/apple/TablePro/Models/Query/QueryTabState.swift index f7b3ea3564..764cd1df8f 100644 --- a/apple/TablePro/Models/Query/QueryTabState.swift +++ b/apple/TablePro/Models/Query/QueryTabState.swift @@ -703,6 +703,12 @@ struct TabDisplayState: Equatable { leaveOutputModeWithoutOutput() } + @MainActor + mutating func replaceActiveServerOutput(_ output: PluginServerOutput) { + activeResultSet?.serverOutput = output + leaveOutputModeWithoutOutput() + } + // New results join after the pinned ones, so a counted name starts there and never repeats a pinned one. @MainActor var pinnedResultCount: Int { diff --git a/apple/TablePro/Views/Main/Child/MainEditorContentView.swift b/apple/TablePro/Views/Main/Child/MainEditorContentView.swift index 5bd064286f..10f531ed79 100644 --- a/apple/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/apple/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -850,6 +850,7 @@ struct MainEditorContentView: View { executionTime: resultSet.executionTime, plan: resultSet.queryPlan, planContext: resultSet.explainPlanContext, + serverOutput: resultSet.serverOutput, tabState: queryPlanViewStates.tabState(forTab: tab.id), planState: queryPlanViewStates.planState( forResultSet: resultSet.id, diff --git a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+Explain.swift b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+Explain.swift index 8a6336c5bb..64385b570e 100644 --- a/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+Explain.swift +++ b/apple/TablePro/Views/Main/Extensions/MainContentCoordinator+Explain.swift @@ -109,6 +109,7 @@ extension MainContentCoordinator { // EXPLAIN ANALYZE and an actual plan run the statement, and a CREATE TABLE AS among them changes the catalog. let ranWrite = request.operationKind(databaseType: conn.type).declaresWrite + let failureOutput = ServerOutputBox() let explainTask = Task { [weak self] in guard let self else { return } do { @@ -117,14 +118,16 @@ extension MainContentCoordinator { route: services.databaseManager.executionRoute(for: scope), cancellation: .cancellableRead(lease) ) { [queryExecutor] driver in - if let step = request.driverStep { - return try await QueryExecutor.fetchExplainPlan( - driver: driver, statement: request.subjectSQL, variantId: step.variantId + try await ServerOutputCapture.running(on: driver, failureOutput: failureOutput) { () async throws -> QueryFetchResult in + if let step = request.driverStep { + return try await QueryExecutor.fetchExplainPlan( + driver: driver, statement: request.subjectSQL, variantId: step.variantId + ) + } + return try await queryExecutor.executeQuery( + driver: driver, sql: request.sql, parameters: nil, rowCap: nil ) } - return try await queryExecutor.executeQuery( - driver: driver, sql: request.sql, parameters: nil, rowCap: nil - ) } if ranWrite { Self.postStatementRan(request.subjectSQL, on: conn) } let rawText = ExplainPlanTextFlattener.flatten(rows: fetchResult.rows) @@ -185,7 +188,8 @@ extension MainContentCoordinator { sql: request.replayableSQL, executionTime: fetchResult.executionTime, anchor: anchor, - planContext: captured.context + planContext: captured.context, + serverOutput: fetchResult.serverOutput )] ) if tab.display.isResultsCollapsed { @@ -235,7 +239,9 @@ extension MainContentCoordinator { ) tabManager.mutate(tabId: tabId) { tab in - tab.execution.errorMessage = error.localizedDescription + tab.execution.errorMessage = ServerOutputCapture.failureMessage( + error.localizedDescription, output: failureOutput.output + ) tab.execution.lastExecutedAt = Date() if tab.display.isResultsCollapsed { tab.display.isResultsCollapsed = false diff --git a/apple/TablePro/Views/QueryPlan/QueryPlanResultView.swift b/apple/TablePro/Views/QueryPlan/QueryPlanResultView.swift index f425beab8a..b142649310 100644 --- a/apple/TablePro/Views/QueryPlan/QueryPlanResultView.swift +++ b/apple/TablePro/Views/QueryPlan/QueryPlanResultView.swift @@ -6,6 +6,7 @@ // import SwiftUI +import TableProPluginKit import TableProUtilities enum QueryPlanViewMode: String, CaseIterable, Identifiable { @@ -13,6 +14,7 @@ enum QueryPlanViewMode: String, CaseIterable, Identifiable { case tree case raw case compare + case output var id: String { rawValue } @@ -22,8 +24,30 @@ enum QueryPlanViewMode: String, CaseIterable, Identifiable { case .tree: return String(localized: "Tree") case .raw: return String(localized: "Raw") case .compare: return String(localized: "Compare") + case .output: return String(localized: "Output") } } + + // Compare needs a readable plan from a known run; Output needs only printed lines, so an unreadable plan has it too. + static func available( + for presentation: QueryPlanPresentation.Kind, + canCompare: Bool, + hasOutput: Bool + ) -> [QueryPlanViewMode] { + let planModes: [QueryPlanViewMode] + switch presentation { + case .parsed: + planModes = canCompare ? [.diagram, .tree, .raw, .compare] : [.diagram, .tree, .raw] + case .rawOnly, .empty: + planModes = [.raw] + } + return hasOutput ? planModes + [.output] : planModes + } + + // The chosen mode belongs to the editor tab and outlives the plan it was chosen on. + static func effective(_ chosen: QueryPlanViewMode, among available: [QueryPlanViewMode]) -> QueryPlanViewMode { + available.contains(chosen) ? chosen : available.first ?? .raw + } } /// What the pane can actually show for this result, resolved once so the view never has to @@ -69,6 +93,7 @@ struct QueryPlanResultView: View { let executionTime: TimeInterval? let plan: QueryPlan? let planContext: QueryPlanContext? + let serverOutput: PluginServerOutput @AppStorage(PreferenceKeys.queryPlanRawFontSize.name) private var fontSize: Double = 13 @AppStorage(PreferenceKeys.queryPlanBarMetric.name) private var storedBarMetric: String = "" @@ -90,6 +115,7 @@ struct QueryPlanResultView: View { executionTime: TimeInterval?, plan: QueryPlan?, planContext: QueryPlanContext? = nil, + serverOutput: PluginServerOutput = .none, tabState: QueryPlanTabState, planState: QueryPlanViewState ) { @@ -97,17 +123,26 @@ struct QueryPlanResultView: View { self.executionTime = executionTime self.plan = plan self.planContext = planContext + self.serverOutput = serverOutput self.tabState = tabState self.planState = planState _comparison = ObservedObject(wrappedValue: tabState.comparison) } - /// Compare is offered only when there is something to compare: a plan the app could read, and a - /// run it knows the identity of. private var availableModes: [QueryPlanViewMode] { - planContext == nil - ? QueryPlanViewMode.allCases.filter { $0 != .compare } - : QueryPlanViewMode.allCases + QueryPlanViewMode.available( + for: presentation.kind, + canCompare: planContext != nil, + hasOutput: !serverOutput.isEmpty + ) + } + + private var shownMode: QueryPlanViewMode { + QueryPlanViewMode.effective(tabState.viewMode, among: availableModes) + } + + private var modeSelection: Binding { + Binding(get: { shownMode }, set: { tabState.viewMode = $0 }) } var body: some View { @@ -123,16 +158,15 @@ struct QueryPlanResultView: View { .task(id: plan?.rootNode.id) { availableMetrics = plan.map(QueryPlanMetricIndex.availableMetrics) ?? [] } - .onChange(of: availableModes) { modes in - guard !modes.contains(tabState.viewMode) else { return } - tabState.viewMode = .diagram - } } @ViewBuilder private var content: some View { - switch presentation { - case .empty: + switch (shownMode, presentation) { + case (.output, _): + ServerOutputView(output: serverOutput) + + case (_, .empty): EmptyStateView( icon: "chart.bar.doc.horizontal", title: String(localized: "No Plan Available"), @@ -140,23 +174,20 @@ struct QueryPlanResultView: View { ) .frame(maxWidth: .infinity, maxHeight: .infinity) - case .rawOnly(let text): + case (_, .rawOnly(let text)): VStack(spacing: 0) { unparsedBanner DDLTextView(ddl: text, fontSize: $fontSize) } - case .parsed(let plan): - switch tabState.viewMode { - case .diagram: - QueryPlanDiagramView(plan: plan, selectedNodeId: $planState.selectedNodeId, viewport: planState.viewport) - case .tree: - QueryPlanTreeView(plan: plan, metric: barMetric, selectedNodeId: $planState.selectedNodeId) - case .raw: - DDLTextView(ddl: rawText, fontSize: $fontSize) - case .compare: - QueryPlanComparisonView(model: comparison) - } + case (.diagram, .parsed(let plan)): + QueryPlanDiagramView(plan: plan, selectedNodeId: $planState.selectedNodeId, viewport: planState.viewport) + case (.tree, .parsed(let plan)): + QueryPlanTreeView(plan: plan, metric: barMetric, selectedNodeId: $planState.selectedNodeId) + case (.raw, .parsed): + DDLTextView(ddl: rawText, fontSize: $fontSize) + case (.compare, .parsed): + QueryPlanComparisonView(model: comparison) } } @@ -178,8 +209,8 @@ struct QueryPlanResultView: View { private var toolbar: some View { HStack(spacing: 12) { - if presentation.plan != nil { - Picker(String(localized: "View Mode"), selection: $tabState.viewMode) { + if availableModes.count > 1 { + Picker(String(localized: "View Mode"), selection: modeSelection) { ForEach(availableModes) { mode in Text(mode.title).tag(mode) } @@ -190,19 +221,19 @@ struct QueryPlanResultView: View { .accessibilityIdentifier("query-plan-mode-picker") } - if tabState.viewMode == .compare { + if shownMode == .compare { baselinePicker } - if tabState.viewMode == .tree { + if shownMode == .tree { metricPicker } - if tabState.viewMode == .raw || presentation.plan == nil { + if shownMode == .raw { fontSizeStepper } - if tabState.viewMode != .compare { + if shownMode != .compare { timings } diff --git a/apple/TableProTests/Core/Services/Execution/BatchStatementRunTests.swift b/apple/TableProTests/Core/Services/Execution/BatchStatementRunTests.swift index 938c15a02a..b20e731cd0 100644 --- a/apple/TableProTests/Core/Services/Execution/BatchStatementRunTests.swift +++ b/apple/TableProTests/Core/Services/Execution/BatchStatementRunTests.swift @@ -19,7 +19,8 @@ struct BatchStatementRunTests { statements: [String] = Self.statements, commitPoints: Set = [], failing: String? = nil, - stopsAfter: Int = .max + stopsAfter: Int = .max, + commitOutput: ServerOutputBox = ServerOutputBox() ) async -> BatchStatementOutcome { let claims = probe ?? ClaimProbe() return await BatchStatementRun.run( @@ -30,7 +31,8 @@ struct BatchStatementRunTests { connectionId: claims.connectionId, gate: claims.gate, failureSQL: { $0 }, - isCommitPoint: { commitPoints.contains($0) } + isCommitPoint: { commitPoints.contains($0) }, + commitOutput: commitOutput ) { statement in if claims.recordExecution() == stopsAfter { claims.stop() } if statement == failing { throw TestError.refused } @@ -318,6 +320,66 @@ struct BatchStatementRunTests { #expect(failure == .commitOutcomeUnknown) } + // MARK: - What the commit printed + + private static let printedAtCommit = PluginServerOutput(lines: ["NOTICE: checked 1"], isTruncated: false) + private static let rollbackWarning = PluginServerOutput( + lines: ["WARNING: there is no transaction in progress"], isTruncated: false + ) + + // A deferred constraint trigger fires at the app's COMMIT, after the last statement's output was read. + @Test("What the app's own commit printed is read once the commit is answered") + func commitOutputIsRead() async { + let driver = TransactionRecordingDriver() + driver.serverOutputs = [Self.printedAtCommit] + let commitOutput = ServerOutputBox() + + let outcome = await Self.run(plan: .appTransaction, driver: driver, commitOutput: commitOutput) + + #expect(commitOutput.output == Self.printedAtCommit) + #expect(driver.outputReads == [[.begin(mode: .readWrite), .commit]]) + guard case .completed = outcome else { + Issue.record("expected a completed run, got \(outcome)") + return + } + } + + @Test("A refused commit has its output read before the rollback, whose own warning is not the user's") + func refusedCommitOutputIsReadBeforeTheRollback() async { + let driver = TransactionRecordingDriver(failsCommit: true) + driver.serverOutputs = [Self.printedAtCommit, Self.rollbackWarning] + let commitOutput = ServerOutputBox() + + let outcome = await Self.run(plan: .appTransaction, driver: driver, commitOutput: commitOutput) + + #expect(commitOutput.output == Self.printedAtCommit) + #expect(driver.outputReads == [[.begin(mode: .readWrite), .commit]]) + #expect(driver.events == [.begin(mode: .readWrite), .commit, .rollback]) + guard case .failed(_, let failure, _) = outcome else { + Issue.record("expected a failed run, got \(outcome)") + return + } + #expect(failure == .commit) + } + + @Test("A completed run shows what the commit printed after its last statement's output") + func commitOutputJoinsTheLastResult() { + var last = QueryResult.empty + last.serverOutput = PluginServerOutput(lines: ["step 2"], isTruncated: false) + + let outcome = BatchStatementOutcome.completed(results: [.empty, last]) + .appendingCommitOutput(Self.printedAtCommit) + + guard case .completed(let results) = outcome else { + Issue.record("expected a completed run, got \(outcome)") + return + } + #expect(results.map { $0.serverOutput } == [ + .none, + PluginServerOutput(lines: ["step 2", "NOTICE: checked 1"], isTruncated: false) + ]) + } + // MARK: - A script's own commit @Test("A script's own COMMIT goes through the commit point, and the batch is stoppable again") @@ -602,6 +664,9 @@ private final class TransactionRecordingDriver: DatabaseDriver, @unchecked Senda var hasLostConnection = false private(set) var events: [TransactionEvent] = [] + // The calls made before each output read, which places the read among them. + private(set) var outputReads: [[TransactionEvent]] = [] + var serverOutputs: [PluginServerOutput] = [] private(set) var cancelCount = 0 private(set) var commitSawCancellation = false private(set) var rollbackSawCancellation = false @@ -652,6 +717,11 @@ private final class TransactionRecordingDriver: DatabaseDriver, @unchecked Senda cancelCount += 1 } + func fetchServerOutput() async throws -> PluginServerOutput { + outputReads.append(events) + return serverOutputs.isEmpty ? .none : serverOutputs.removeFirst() + } + func execute(query: String) async throws -> QueryResult { executedStatements += 1 if executedStatements == 1, let hook = whileExecuting { await MainActor.run { hook() } } diff --git a/apple/TableProTests/Core/Services/Query/ServerOutputCaptureTests.swift b/apple/TableProTests/Core/Services/Query/ServerOutputCaptureTests.swift index 2497834da1..928191671a 100644 --- a/apple/TableProTests/Core/Services/Query/ServerOutputCaptureTests.swift +++ b/apple/TableProTests/Core/Services/Query/ServerOutputCaptureTests.swift @@ -2,11 +2,12 @@ // ServerOutputCaptureTests.swift // TableProTests // -// The server hands printed lines to whoever asks next, so output a statement left unread is reported under the one -// after it. These pin the read after every statement, the failed one included, and what a failure shows. +// The server hands printed lines to whoever asks next. These pin the read before every statement, which discards +// what other work left, the read after it, the failed one included, and what a failure shows. // import Foundation +import os @testable import TablePro import TableProPluginKit import Testing @@ -15,6 +16,7 @@ struct ServerOutputCaptureTests { private struct StatementFailed: Error {} private static let printed = PluginServerOutput(lines: ["Hello from PL/SQL", ""], isTruncated: false) + private static let leftOver = PluginServerOutput(lines: ["NOTICE: audit row written"], isTruncated: false) private static func result() -> QueryResult { QueryResult(columns: [], columnTypes: [], rows: [], rowsAffected: 0, executionTime: 0, error: nil) @@ -44,11 +46,11 @@ struct ServerOutputCaptureTests { } } #expect(box.output == Self.printed) - #expect(driver.fetchServerOutputCallCount == 1) + #expect(driver.fetchServerOutputCallCount == 2) } - @Test("A cancelled statement sends nothing more to the server") - func cancellationReadsNothing() async { + @Test("A cancelled statement sends nothing more to the server after it") + func cancellationReadsNothingAfter() async { let driver = MockDatabaseDriver() let box = ServerOutputBox() @@ -57,7 +59,59 @@ struct ServerOutputCaptureTests { throw CancellationError() } } - #expect(driver.fetchServerOutputCallCount == 0) + #expect(driver.fetchServerOutputCallCount == 1) + } + + @Test("What the session printed before the statement is not reported as its output") + func earlierOutputIsDiscarded() async throws { + let session = PrintingSession(printed: [Self.leftOver, Self.printed]) + + let result = try await ServerOutputCapture.running(on: session.driver, failureOutput: ServerOutputBox()) { + Self.result() + } + + #expect(result.serverOutput == Self.printed) + #expect(session.plugin.reads == 2) + } + + @Test("A failure reports only what the failed statement printed") + func failureDiscardsEarlierOutput() async { + let session = PrintingSession(printed: [Self.leftOver, Self.printed]) + let box = ServerOutputBox() + + await #expect(throws: StatementFailed.self) { + _ = try await ServerOutputCapture.running(on: session.driver, failureOutput: box) { () throws -> QueryResult in + throw StatementFailed() + } + } + #expect(box.output == Self.printed) + } + + // Oracle dials a new session after a Stop closed the old one, so the statement would still run there. + @Test("A Stop during the read before the statement runs no statement") + func stopDuringTheEarlierReadRunsNothing() async { + let session = PrintingSession(printed: [Self.leftOver]) + session.plugin.whileReading = { withUnsafeCurrentTask { $0?.cancel() } } + let ran = OSAllocatedUnfairLock(initialState: false) + + let run = Task { + try await ServerOutputCapture.running(on: session.driver, failureOutput: ServerOutputBox()) { () throws -> QueryResult in + ran.withLock { $0 = true } + return Self.result() + } + } + + await #expect(throws: CancellationError.self) { _ = try await run.value } + #expect(ran.withLock { $0 } == false) + } + + @Test("Appending keeps the lines in order and stays cut short once either part was") + func appendingJoinsTheLines() { + let first = PluginServerOutput(lines: ["step 1"], isTruncated: false) + let later = PluginServerOutput(lines: ["checked 1", "checked 2"], isTruncated: true) + + #expect(first.appending(later) == PluginServerOutput(lines: ["step 1", "checked 1", "checked 2"], isTruncated: true)) + #expect(first.appending(.none) == first) } @Test("A failure lists what the statement printed before it failed") @@ -86,3 +140,30 @@ struct ServerOutputCaptureTests { }) } } + +// Each read takes what the session printed since the last one, as a real session's output is consumed. +private struct PrintingSession { + let plugin: SequencedOutputPlugin + let driver: DatabaseDriver + + init(printed: [PluginServerOutput]) { + plugin = SequencedOutputPlugin(printed: printed) + driver = PluginDriverAdapter(connection: TestFixtures.makeConnection(), pluginDriver: plugin) + } +} + +private final class SequencedOutputPlugin: PluginDriverStub, @unchecked Sendable { + private var printed: [PluginServerOutput] + private(set) var reads = 0 + var whileReading: (() -> Void)? + + init(printed: [PluginServerOutput]) { + self.printed = printed + } + + func fetchServerOutput() async throws -> PluginServerOutput { + reads += 1 + whileReading?() + return printed.isEmpty ? .none : printed.removeFirst() + } +} diff --git a/apple/TableProTests/Views/Main/FetchAllQueryTaskTests.swift b/apple/TableProTests/Views/Main/FetchAllQueryTaskTests.swift index 3db6076a36..4e18a0aadb 100644 --- a/apple/TableProTests/Views/Main/FetchAllQueryTaskTests.swift +++ b/apple/TableProTests/Views/Main/FetchAllQueryTaskTests.swift @@ -4,6 +4,7 @@ import Foundation @testable import TablePro +import TableProPluginKit import TableProTestSupport import Testing @@ -52,6 +53,49 @@ struct FetchAllQueryTaskTests { #expect(coordinator.tabExecution.isBusy(tab.id) == false) #expect(tabManager.tabs.first { $0.id == tab.id }?.pagination.isLoadingMore == false) } + + @Test("Fetch All on a query tab shows what its own run printed, not the first run's or anything before it") + func fetchAllReplacesTheOutput() async { + let firstRun = PluginServerOutput(lines: ["NOTICE: page 1"], isTruncated: false) + let leftOver = PluginServerOutput(lines: ["NOTICE: audit row written"], isTruncated: false) + let refetched = PluginServerOutput(lines: ["NOTICE: page 1", "NOTICE: page 2"], isTruncated: false) + let connection = TestFixtures.makeConnection() + let driver = GatedQueryDriver(connection: connection) + driver.serverOutputs = [leftOver, refetched] + await driver.release() + var session = ConnectionSession(connection: connection, driver: driver) + session.status = .connected + session.browseDatabase = connection.database + DatabaseManager.shared.injectSession(session, for: connection.id) + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: connection, + tabManager: tabManager, + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + var tab = QueryTab(title: "Query", query: "SELECT noisy()", tabType: .query) + let shown = ResultSet(label: "Result") + shown.serverOutput = firstRun + tab.display.replaceUnpinnedResults(with: [shown]) + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + + coordinator.paginationCoordinator.performFetchAll( + tabId: tab.id, + baseQuery: "SELECT noisy()", + scope: DatabaseScope(connectionId: connection.id, database: connection.database, schema: nil) + ) + guard let handle = coordinator.queryTasks.task(for: tab.id) else { + Issue.record("the fetch installed no query handle") + return + } + await handle.value + + #expect(tabManager.tabs.first { $0.id == tab.id }?.display.activeResultSet?.serverOutput == refetched) + } } /// Answers `executeUserQuery` only once the test lets it, and never by raising: a cancel has to find @@ -62,6 +106,7 @@ private final class GatedQueryDriver: DatabaseDriver, @unchecked Sendable { var serverVersion: String? { nil } private let answer = AsyncGate() + var serverOutputs: [PluginServerOutput] = [] init(connection: DatabaseConnection) { self.connection = connection @@ -82,6 +127,10 @@ private final class GatedQueryDriver: DatabaseDriver, @unchecked Sendable { return Self.emptyResult } + func fetchServerOutput() async throws -> PluginServerOutput { + serverOutputs.isEmpty ? .none : serverOutputs.removeFirst() + } + func connect() async throws {} func disconnect() {} func testConnection() async throws -> Bool { true } diff --git a/apple/TableProTests/Views/QueryPlan/QueryPlanPresentationTests.swift b/apple/TableProTests/Views/QueryPlan/QueryPlanPresentationTests.swift index d8df9e75eb..f9a9f9bf08 100644 --- a/apple/TableProTests/Views/QueryPlan/QueryPlanPresentationTests.swift +++ b/apple/TableProTests/Views/QueryPlan/QueryPlanPresentationTests.swift @@ -8,6 +8,7 @@ import Foundation @testable import TablePro +import TableProPluginKit import Testing struct QueryPlanPresentationTests { @@ -61,8 +62,47 @@ struct QueryPlanPresentationTests { for mode in QueryPlanViewMode.allCases { #expect(!mode.title.isEmpty) } - #expect(QueryPlanViewMode.allCases.count == 4) + #expect(QueryPlanViewMode.allCases.count == 5) #expect(QueryPlanViewMode.allCases.contains(.compare)) + #expect(QueryPlanViewMode.allCases.contains(.output)) + } + + @Test("A parsed plan offers Output only when its statement printed") + func parsedPlanOffersOutputWithOutput() { + #expect(QueryPlanViewMode.available(for: .parsed, canCompare: false, hasOutput: false) == [.diagram, .tree, .raw]) + #expect(QueryPlanViewMode.available(for: .parsed, canCompare: false, hasOutput: true) + == [.diagram, .tree, .raw, .output]) + #expect(QueryPlanViewMode.available(for: .parsed, canCompare: true, hasOutput: true) + == [.diagram, .tree, .raw, .compare, .output]) + } + + // PostgreSQL's typed EXPLAIN ANALYZE is text, which no parser reads, and it is the one that runs the statement. + @Test("A plan the app could not read still offers Output beside Raw when its statement printed", arguments: [ + QueryPlanPresentation.Kind.rawOnly, .empty + ]) + func unreadPlanOffersOutputWithOutput(kind: QueryPlanPresentation.Kind) { + #expect(QueryPlanViewMode.available(for: kind, canCompare: true, hasOutput: false) == [.raw]) + #expect(QueryPlanViewMode.available(for: kind, canCompare: true, hasOutput: true) == [.raw, .output]) + } + + // The mode belongs to the editor tab, so Output chosen on one plan is still chosen when the next has none. + @Test("A chosen mode the plan does not offer shows the plan's first mode instead") + func chosenModeFallsBackWhenNotOffered() { + #expect(QueryPlanViewMode.effective(.output, among: [.diagram, .tree, .raw]) == .diagram) + #expect(QueryPlanViewMode.effective(.output, among: [.raw]) == .raw) + #expect(QueryPlanViewMode.effective(.compare, among: [.diagram, .tree, .raw, .output]) == .diagram) + #expect(QueryPlanViewMode.effective(.tree, among: [.diagram, .tree, .raw]) == .tree) + #expect(QueryPlanViewMode.effective(.output, among: [.raw, .output]) == .output) + } + + @Test("A plan's result set keeps what its statement printed") + @MainActor + func explainResultSetCarriesTheOutput() { + let printed = PluginServerOutput(lines: ["NOTICE: step 1 done"], isTruncated: false) + let plan = ExplainResultSetFactory.make( + rawText: "Seq Scan", plan: nil, sql: "EXPLAIN ANALYZE SELECT demo()", executionTime: 0.1, serverOutput: printed + ) + #expect(plan.serverOutput == printed) } @Test("An explain result set is recognised by its raw text, not by a parsed plan") diff --git a/apple/scripts/probes/check-postgresql-notices.sh b/apple/scripts/probes/check-postgresql-notices.sh new file mode 100755 index 0000000000..6357e27e05 --- /dev/null +++ b/apple/scripts/probes/check-postgresql-notices.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# +# Check the libpq notice facts LibPQServerMessageSink and LibPQNoticeOutput rely on, against the libpq the plugin +# links and a live server. +# +# - RAISE NOTICE, WARNING and INFO reach the receiver as ": \n" in PQresultErrorMessage. +# - DETAIL and HINT follow as lines of their own, and a newline inside the message stays. +# - PL/pgSQL sets CONTEXT, and libpq's default context visibility leaves it out of a notice's text. +# - A notice raised before an error still arrives. +# - libpq's own notices carry no SQLSTATE. +# +# Usage: +# scripts/probes/check-postgresql-notices.sh [conninfo] +# +# conninfo defaults to "host=localhost dbname=postgres"; PGUSER and PGPASSWORD apply. Expects English server +# messages. Needs apple/Libs from download-libs.sh. Skips (exit 3) without them or without a server. Exits non-zero +# when a fact no longer holds. + +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +LIBS="$ROOT/Libs" +INCLUDE="$ROOT/Plugins/PostgreSQLDriverPlugin/CLibPQ/include" +CONNINFO="${1:-host=localhost dbname=postgres}" + +[ -f "$LIBS/libpq_arm64.a" ] || { echo "skip: run scripts/download-libs.sh" >&2; exit 3; } + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +cat >"$WORK/probe.c" <<'EOF' +#include +#include +#include "libpq-fe.h" + +static int failures = 0; +static char text[4096]; +static int notices = 0; +static int without_sqlstate = 0; +static int with_context_field = 0; + +static void receiver(void *arg, const PGresult *res) { + (void)arg; + size_t used = strlen(text); + snprintf(text + used, sizeof text - used, "%s", PQresultErrorMessage(res)); + notices++; + if (!PQresultErrorField(res, PG_DIAG_SQLSTATE)) without_sqlstate++; + if (PQresultErrorField(res, PG_DIAG_CONTEXT)) with_context_field++; +} + +static void reset(void) { + text[0] = '\0'; + notices = without_sqlstate = with_context_field = 0; +} + +static void expect(int ok, const char *fact) { + printf("%s %s\n", ok ? "ok " : "FAIL", fact); + if (!ok) { + printf(" received: \"%s\"\n", text); + failures++; + } +} + +static void expect_text(PGconn *conn, const char *sql, const char *expected, const char *fact) { + reset(); + PQclear(PQexec(conn, sql)); + expect(strcmp(text, expected) == 0 && without_sqlstate == 0, fact); +} + +int main(int argc, char **argv) { + PGconn *conn = PQconnectdb(argv[1]); + if (PQstatus(conn) != CONNECTION_OK) { + fprintf(stderr, "skip: %s", PQerrorMessage(conn)); + return 3; + } + PQsetNoticeReceiver(conn, receiver, NULL); + + expect_text(conn, "DO $$ BEGIN RAISE NOTICE 'step 1 done'; END $$", + "NOTICE: step 1 done\n", "RAISE NOTICE is \"NOTICE: \" plus one newline, with a SQLSTATE"); + expect_text(conn, "DO $$ BEGIN RAISE WARNING 'w'; RAISE INFO 'i'; END $$", + "WARNING: w\nINFO: i\n", "RAISE WARNING and RAISE INFO carry their own severity"); + expect_text(conn, "DO $$ BEGIN RAISE NOTICE 'extras' USING DETAIL = 'the detail', HINT = 'the hint'; END $$", + "NOTICE: extras\nDETAIL: the detail\nHINT: the hint\n", "DETAIL and HINT are lines of their own"); + expect_text(conn, "DO $$ BEGIN RAISE NOTICE E'line1\\nline2'; END $$", + "NOTICE: line1\nline2\n", "a newline inside the message stays"); + expect(with_context_field == 1 && strstr(text, "CONTEXT:") == NULL, + "PL/pgSQL sets CONTEXT and the text leaves it out"); + + reset(); + PGresult *failed = PQexec(conn, "DO $$ BEGIN RAISE NOTICE 'before the error'; RAISE EXCEPTION 'boom'; END $$"); + expect(PQresultStatus(failed) == PGRES_FATAL_ERROR && strcmp(text, "NOTICE: before the error\n") == 0, + "a notice raised before an error arrives, and the error does not"); + PQclear(failed); + + PGresult *one = PQexec(conn, "SELECT 1"); + reset(); + (void)PQgetvalue(one, 5, 0); + PQclear(one); + expect(notices == 1 && without_sqlstate == 1 && strncmp(text, "NOTICE: ", 9) != 0, + "libpq's own notice has no SQLSTATE and no severity prefix"); + + PQfinish(conn); + return failures == 0 ? 0 : 1; +} +EOF + +clang -I"$INCLUDE" -o "$WORK/probe" "$WORK/probe.c" \ + "$LIBS/libpq_arm64.a" "$LIBS/libpgcommon_arm64.a" "$LIBS/libpgport_arm64.a" \ + "$LIBS/libssl_arm64.a" "$LIBS/libcrypto_arm64.a" -lz -framework GSS || exit 1 + +"$WORK/probe" "$CONNINFO" diff --git a/core/crates/core/src/result_event.rs b/core/crates/core/src/result_event.rs index 576b3a54eb..006392fb4a 100644 --- a/core/crates/core/src/result_event.rs +++ b/core/crates/core/src/result_event.rs @@ -58,6 +58,11 @@ pub enum ResultEvent { rows_affected: Option, }, Notice(ServerNotice), + /// The run raised more notices than it keeps. `retained` were sent + /// before it; the rest are gone. + NoticeLimitReached { + retained: u64, + }, } /// How a run ended. diff --git a/core/crates/drivers/postgres/src/editor/actor.rs b/core/crates/drivers/postgres/src/editor/actor.rs index 09c8169644..fda5f7a693 100644 --- a/core/crates/drivers/postgres/src/editor/actor.rs +++ b/core/crates/drivers/postgres/src/editor/actor.rs @@ -1,11 +1,11 @@ use std::sync::Arc; use std::time::Duration; -use tablepro_core::{DriverError, LossPhase, ResultSink, RunOptions, RunSummary, UserSql}; +use tablepro_core::{DriverError, LossPhase, ResultSink, RunOptions, RunSummary, ServerNotice, UserSql}; use tokio::sync::{mpsc, oneshot}; -use tokio_postgres::error::DbError; use tokio_util::sync::CancellationToken; +use super::notice_queue::NoticeSender; use super::session_core::{SessionCore, drive}; use super::session_status::SessionStatus; use crate::physical::PgRawConnection; @@ -28,7 +28,7 @@ pub(crate) enum Command { pub(crate) async fn session_actor( mut core: SessionCore, connection: PgRawConnection, - notices: mpsc::UnboundedSender, + notices: NoticeSender, mut commands: mpsc::Receiver, status: Arc, shutdown: CancellationToken, diff --git a/core/crates/drivers/postgres/src/editor/editor_session.rs b/core/crates/drivers/postgres/src/editor/editor_session.rs index 7df7283131..2e1a692aa2 100644 --- a/core/crates/drivers/postgres/src/editor/editor_session.rs +++ b/core/crates/drivers/postgres/src/editor/editor_session.rs @@ -12,6 +12,7 @@ use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; use super::actor::{Command, session_actor}; +use super::notice_queue::{NOTICES_PER_RUN, notice_channel}; use super::session_core::SessionCore; use super::session_status::SessionStatus; use crate::physical::PgEndpoint; @@ -43,7 +44,7 @@ impl PgEditorSession { let (client, connection) = endpoint.open(&cancel).await?; let database = endpoint.database().map(str::to_owned); let close_timeout = endpoint.liveness().close_timeout; - let (notice_sender, notices) = mpsc::unbounded_channel(); + let (notice_sender, notices) = notice_channel(NOTICES_PER_RUN); // PostgreSQL has defaulted to on since 9.1; the server reports // its own value at startup and on every change. let conforming = Arc::new(AtomicBool::new(true)); diff --git a/core/crates/drivers/postgres/src/editor/mod.rs b/core/crates/drivers/postgres/src/editor/mod.rs index 570d453aeb..7c351a488d 100644 --- a/core/crates/drivers/postgres/src/editor/mod.rs +++ b/core/crates/drivers/postgres/src/editor/mod.rs @@ -6,6 +6,7 @@ mod actor; mod cursor_flight; mod editor_dispatch; mod editor_session; +mod notice_queue; mod run; mod session_core; mod session_status; diff --git a/core/crates/drivers/postgres/src/editor/notice_queue.rs b/core/crates/drivers/postgres/src/editor/notice_queue.rs new file mode 100644 index 0000000000..f9089d5504 --- /dev/null +++ b/core/crates/drivers/postgres/src/editor/notice_queue.rs @@ -0,0 +1,150 @@ +//! The notices a session's connection hands to its runs, at most a run's +//! worth at a time. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + +use tokio::sync::mpsc::{self, UnboundedReceiver, UnboundedSender}; + +/// Notices one run keeps. A loop that raises one per row would otherwise +/// grow the queue, and the results, for as long as it runs. +pub(crate) const NOTICES_PER_RUN: u64 = 10_000; + +pub(crate) fn notice_channel(limit: u64) -> (NoticeSender, NoticeQueue) { + let budget = Arc::new(Budget { + limit, + admitted: AtomicU64::new(0), + dropped: AtomicBool::new(false), + }); + let (sender, receiver) = mpsc::unbounded_channel(); + ( + NoticeSender { + sender, + budget: Arc::clone(&budget), + }, + NoticeQueue { receiver, budget }, + ) +} + +/// Counted where the connection reads a notice, so one past the budget is +/// dropped there and never queued. +struct Budget { + limit: u64, + admitted: AtomicU64, + dropped: AtomicBool, +} + +/// The connection's end. +pub(crate) struct NoticeSender { + sender: UnboundedSender, + budget: Arc, +} + +impl NoticeSender { + pub(crate) fn send(&self, notice: T) { + let admitted = self + .budget + .admitted + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |count| { + (count < self.budget.limit).then(|| count.saturating_add(1)) + }) + .is_ok(); + if admitted { + let _ = self.sender.send(notice); + } else { + self.budget.dropped.store(true, Ordering::SeqCst); + } + } +} + +/// The session's end. +pub(crate) struct NoticeQueue { + receiver: UnboundedReceiver, + budget: Arc, +} + +impl NoticeQueue { + pub(crate) fn start_run(&self) { + self.budget.admitted.store(0, Ordering::SeqCst); + self.budget.dropped.store(false, Ordering::SeqCst); + } + + pub(crate) fn next(&mut self) -> Option { + self.receiver.try_recv().ok() + } + + pub(crate) fn discard(&mut self) { + while self.receiver.try_recv().is_ok() {} + } + + /// Whether the connection dropped a notice since the run started. + /// Every notice it queued before the first drop is already queued. + pub(crate) fn dropped(&self) -> bool { + self.budget.dropped.load(Ordering::SeqCst) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn drain(queue: &mut NoticeQueue) -> Vec { + std::iter::from_fn(|| queue.next()).collect() + } + + #[test] + fn notices_past_the_budget_are_dropped_and_said_to_be() { + let (sender, mut queue) = notice_channel(3); + + for notice in 1..=5 { + sender.send(notice); + } + + assert_eq!(drain(&mut queue), [1, 2, 3]); + assert!(queue.dropped()); + } + + #[test] + fn a_run_within_the_budget_drops_nothing() { + let (sender, mut queue) = notice_channel(3); + + for notice in 1..=3 { + sender.send(notice); + } + + assert_eq!(drain(&mut queue), [1, 2, 3]); + assert!(!queue.dropped()); + } + + #[test] + fn a_new_run_starts_with_the_whole_budget() { + let (sender, mut queue) = notice_channel(2); + for notice in 1..=4 { + sender.send(notice); + } + drain(&mut queue); + + queue.start_run(); + sender.send(10); + sender.send(11); + + assert!(!queue.dropped()); + assert_eq!(drain(&mut queue), [10, 11]); + sender.send(12); + assert!(queue.dropped()); + assert!(queue.next().is_none()); + } + + #[test] + fn discarded_notices_still_spend_the_runs_budget() { + let (sender, mut queue) = notice_channel(2); + + sender.send(1); + queue.discard(); + sender.send(2); + sender.send(3); + + assert_eq!(drain(&mut queue), [2]); + assert!(queue.dropped()); + } +} diff --git a/core/crates/drivers/postgres/src/editor/run.rs b/core/crates/drivers/postgres/src/editor/run.rs index cca1a904cb..92d2185fe9 100644 --- a/core/crates/drivers/postgres/src/editor/run.rs +++ b/core/crates/drivers/postgres/src/editor/run.rs @@ -4,9 +4,7 @@ use std::sync::atomic::Ordering; use futures_util::StreamExt; use tablepro_core::sql_syntax::SqlGrammar; use tablepro_core::sql_syntax::script::{LexicalSettings, ScriptPlan, ScriptToken}; -use tablepro_core::{ - DriverError, ResultEvent, ResultSink, RowLimit, RunEnd, ServerCode, ServerNotice, SinkFlow, Truncation, Value, -}; +use tablepro_core::{DriverError, ResultEvent, ResultSink, RowLimit, RunEnd, SinkFlow, Truncation, Value}; use tokio_postgres::SimpleQueryMessage; use super::cursor_flight::{ @@ -31,6 +29,10 @@ enum Role { /// The driver's own statement: its results and notices are not the /// user's data. Control, + /// The driver's own statement around the user's, as the cursor's + /// DECLARE and the COMMIT that ends it: no rows of its own, but what + /// the server says while it runs belongs to the user's statement. + Wrapper, User { class: StatementClass, cursor: bool, @@ -44,6 +46,8 @@ pub(crate) struct Run<'a> { watch: StopWatch, limit: RowLimit, sink_gone: bool, + notices_sent: u64, + notice_limit_reported: bool, } /// The result set an exchange is reading. @@ -69,6 +73,8 @@ impl<'a> Run<'a> { watch, limit, sink_gone: false, + notices_sent: 0, + notice_limit_reported: false, } } @@ -175,12 +181,18 @@ impl<'a> Run<'a> { if let Some(error) = self.exchange(open, Role::Control).await? { return Ok(Some(error)); } - if let Some(error) = self.exchange(&declare(text), Role::Control).await? { - let error = correct_position(error); - if needs_plain_statement(&error) && !self.stopped() { - self.abandon_cursor(owner, true).await?; - return self.simple(text, class, fallback(owner)).await; - } + let declared = self.read(&declare(text), Role::Wrapper).await?.map(correct_position); + if declared + .as_ref() + .is_some_and(|error| needs_plain_statement(error) && !self.stopped()) + { + // The statement raises the same notices again when it goes plain. + self.core.discard_notices(); + self.abandon_cursor(owner, true).await?; + return self.simple(text, class, fallback(owner)).await; + } + self.forward_notices().await; + if let Some(error) = declared { self.abandon_cursor(owner, false).await?; return Ok(Some(error)); } @@ -192,9 +204,10 @@ impl<'a> Run<'a> { return Ok(Some(error)); } self.exchange(CLOSE, Role::Control).await?; + // Deferred triggers fire at the COMMIT, for the user's statement. let closed = match owner { - CursorOwner::Driver { .. } => self.exchange("COMMIT", Role::Control).await?, - CursorOwner::Savepoint => self.exchange(RELEASE_SAVEPOINT, Role::Control).await?, + CursorOwner::Driver { .. } => self.exchange("COMMIT", Role::Wrapper).await?, + CursorOwner::Savepoint => self.exchange(RELEASE_SAVEPOINT, Role::Wrapper).await?, }; Ok(closed) } @@ -251,13 +264,23 @@ impl<'a> Run<'a> { Err(Lost) } - /// Send one message and read everything it returns, keeping the - /// first error. A stop that fires meanwhile sends the cancel request - /// and keeps reading, because the statement's answer, its 57014 or - /// its rows, is still on the wire. + /// Send one message and read everything it returns, keeping the first + /// error. Its notices reach the sink unless it is the driver's own. async fn exchange(&mut self, sql: &str, role: Role) -> Result, Lost> { + let error = self.read(sql, role).await?; + match role { + Role::User { .. } | Role::Wrapper => self.forward_notices().await, + Role::Control => self.core.discard_notices(), + } + Ok(error) + } + + /// An exchange that leaves its notices queued. A stop that fires + /// meanwhile sends the cancel request and keeps reading, because the + /// statement's answer, its 57014 or its rows, is still on the wire. + async fn read(&mut self, sql: &str, role: Role) -> Result, Lost> { self.watch.settle().await; - if self.stopped() && role != Role::Control { + if self.stopped() && matches!(role, Role::User { .. }) { return Ok(Some(DriverError::Cancelled)); } let stream = match self.core.client.simple_query_raw(sql).await { @@ -293,10 +316,6 @@ impl<'a> Run<'a> { } } } - match role { - Role::User { .. } => self.forward_notices().await, - Role::Control => self.core.discard_notices(), - } Ok(first_error) } @@ -376,12 +395,17 @@ impl<'a> Run<'a> { } pub(crate) async fn forward_notices(&mut self) { + // Read first: a drop comes after every notice queued before it. + let dropped = self.core.notices_dropped(); while let Some(notice) = self.core.next_notice() { - self.accept(ResultEvent::Notice(ServerNotice { - severity: notice.severity().to_owned(), - message: notice.message().to_owned(), - code: Some(ServerCode::SqlState(notice.code().code().to_owned())), - })) + self.notices_sent += 1; + self.accept(ResultEvent::Notice(notice)).await; + } + if dropped && !self.notice_limit_reported { + self.notice_limit_reported = true; + self.accept(ResultEvent::NoticeLimitReached { + retained: self.notices_sent, + }) .await; } } diff --git a/core/crates/drivers/postgres/src/editor/session_core.rs b/core/crates/drivers/postgres/src/editor/session_core.rs index 91846a3295..bfdb6e5d42 100644 --- a/core/crates/drivers/postgres/src/editor/session_core.rs +++ b/core/crates/drivers/postgres/src/editor/session_core.rs @@ -3,13 +3,13 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::task::Poll; use std::time::Duration; -use tablepro_core::{ResultSink, RunOptions, RunSummary, UserSql}; -use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; +use tablepro_core::{ResultSink, RunOptions, RunSummary, ServerCode, ServerNotice, UserSql}; use tokio::time::Instant; use tokio_postgres::error::DbError; use tokio_postgres::{AsyncMessage, CancelToken, Client}; use tokio_util::sync::CancellationToken; +use super::notice_queue::{NoticeQueue, NoticeSender}; use super::run::Run; use super::stop_watch::StopWatch; use super::transaction_tracker::TransactionTracker; @@ -27,7 +27,7 @@ pub(crate) struct SessionCore { pub(crate) tracker: TransactionTracker, pub(crate) read_only: bool, pub(crate) conforming: Arc, - notices: UnboundedReceiver, + notices: NoticeQueue, statement_timeout: Option, grace: Duration, } @@ -38,7 +38,7 @@ impl SessionCore { endpoint: Arc, read_only: bool, statement_timeout: Option, - notices: UnboundedReceiver, + notices: NoticeQueue, conforming: Arc, ) -> Self { let cancel_token = client.cancel_token(); @@ -60,12 +60,16 @@ impl SessionCore { self.statement_timeout } - pub(crate) fn next_notice(&mut self) -> Option { - self.notices.try_recv().ok() + pub(crate) fn next_notice(&mut self) -> Option { + self.notices.next() } pub(crate) fn discard_notices(&mut self) { - while self.notices.try_recv().is_ok() {} + self.notices.discard(); + } + + pub(crate) fn notices_dropped(&self) -> bool { + self.notices.dropped() } pub(crate) async fn run( @@ -76,6 +80,7 @@ impl SessionCore { shutdown: &CancellationToken, ) -> Result { let started = Instant::now(); + self.notices.start_run(); let deadline = options.deadline.map(|deadline| started + deadline); let watch = StopWatch::new(options.cancel, shutdown.clone(), deadline, self.grace); let mut run = Run::new(self, sink, watch, options.row_limit); @@ -100,7 +105,7 @@ impl SessionCore { /// split. Resolves when the connection ends. pub(crate) async fn drive( mut connection: PgRawConnection, - notices: UnboundedSender, + notices: NoticeSender, conforming: Arc, ) { std::future::poll_fn(|cx| { @@ -112,9 +117,7 @@ pub(crate) async fn drive( ); match polled { Poll::Pending => return Poll::Pending, - Poll::Ready(Some(Ok(AsyncMessage::Notice(notice)))) => { - let _ = notices.send(notice); - } + Poll::Ready(Some(Ok(AsyncMessage::Notice(notice)))) => notices.send(kept_notice(¬ice)), Poll::Ready(Some(Ok(_))) => {} Poll::Ready(Some(Err(_)) | None) => return Poll::Ready(()), } @@ -122,3 +125,44 @@ pub(crate) async fn drive( }) .await; } + +/// The longest notice message a run keeps, in bytes: a run holds up to +/// `NOTICES_PER_RUN` of them until it reads them. +const NOTICE_MESSAGE_BYTES: usize = 32_767; + +fn kept_notice(notice: &DbError) -> ServerNotice { + ServerNotice { + severity: notice.severity().to_owned(), + message: clipped(notice.message()), + code: Some(ServerCode::SqlState(notice.code().code().to_owned())), + } +} + +fn clipped(message: &str) -> String { + if message.len() <= NOTICE_MESSAGE_BYTES { + return message.to_owned(); + } + let mut end = NOTICE_MESSAGE_BYTES; + while !message.is_char_boundary(end) { + end -= 1; + } + format!("{}\u{2026}", &message[..end]) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_message_within_the_limit_is_kept_whole() { + let message = "x".repeat(NOTICE_MESSAGE_BYTES); + assert_eq!(clipped(&message), message); + } + + #[test] + fn a_longer_message_is_cut_on_a_character_boundary() { + let message = format!("{}\u{e9}tail", "x".repeat(NOTICE_MESSAGE_BYTES - 1)); + let kept = clipped(&message); + assert_eq!(kept, format!("{}\u{2026}", "x".repeat(NOTICE_MESSAGE_BYTES - 1))); + } +} diff --git a/core/crates/drivers/postgres/tests/integration/editor_session.rs b/core/crates/drivers/postgres/tests/integration/editor_session.rs index 63eb936270..b8389adb40 100644 --- a/core/crates/drivers/postgres/tests/integration/editor_session.rs +++ b/core/crates/drivers/postgres/tests/integration/editor_session.rs @@ -495,6 +495,114 @@ async fn notices_reach_the_sink() { ); } +fn notices(events: &Events) -> Vec { + events + .all() + .into_iter() + .filter_map(|event| match event { + ResultEvent::Notice(notice) => Some(notice.message), + _ => None, + }) + .collect() +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn notices_past_the_run_limit_end_in_one_limit_event_and_the_next_run_starts_afresh() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + + let flood = run_ok( + session.as_ref(), + "DO $$ BEGIN FOR i IN 1..10005 LOOP RAISE NOTICE 'row %', i; END LOOP; END $$", + ) + .await; + let next = run_ok(session.as_ref(), "DO $$ BEGIN RAISE NOTICE 'after'; END $$").await; + + let all = flood.all(); + let is_limit = |event: &ResultEvent| matches!(event, ResultEvent::NoticeLimitReached { .. }); + let limit = all.iter().position(is_limit).expect("the limit event"); + assert_eq!(all[limit], ResultEvent::NoticeLimitReached { retained: 10_000 }); + assert_eq!(all.iter().filter(|event| is_limit(event)).count(), 1); + assert!( + !all[limit..].iter().any(|event| matches!(event, ResultEvent::Notice(_))), + "a notice came after the limit" + ); + assert_eq!(notices(&flood).len(), 10_000); + assert_eq!(notices(&flood).last().map(String::as_str), Some("row 10000")); + assert_eq!(notices(&next), ["after"]); + assert!(!next.all().iter().any(is_limit)); +} + +/// A parse-time warning arrives on the cursor's DECLARE, not on the FETCH. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_row_limited_select_keeps_the_warning_its_declare_raised() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + run_ok(session.as_ref(), "SET standard_conforming_strings = off").await; + + let events = run_ok(session.as_ref(), "SELECT 'a\\\\b' AS x").await; + + let all = events.all(); + let warning = all + .iter() + .position(|event| matches!(event, ResultEvent::Notice(notice) if notice.message.starts_with("nonstandard use of \\\\"))) + .unwrap_or_else(|| panic!("no warning: {all:?}")); + let started = all + .iter() + .position(|event| matches!(event, ResultEvent::ResultSetStarted { .. })) + .expect("the result"); + assert!(warning < started, "{all:?}"); + assert_eq!(notices(&events).len(), 1, "{all:?}"); + assert_eq!(texts(&events), ["a\\b"]); +} + +/// SELECT INTO cannot be a cursor, so it goes again as a plain statement, +/// which raises its warning a second time. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_statement_that_falls_back_from_the_cursor_reports_its_warning_once() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + let session = session(connection.as_ref()).await; + run_ok(session.as_ref(), "SET standard_conforming_strings = off").await; + + let events = run_ok(session.as_ref(), "SELECT 'a\\\\b' AS x INTO copied").await; + + assert_eq!(notices(&events).len(), 1, "{:?}", events.all()); +} + +/// A deferred constraint trigger fires at the COMMIT that ends the +/// cursor's own transaction, for the statement that wrote the row. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[ignore = "requires docker"] +async fn a_notice_raised_at_the_cursors_commit_reaches_the_sink() { + let fixture = start().await; + let connection = connect(&fixture, false).await; + setup( + connection.as_ref(), + &[ + "CREATE TABLE checked (id int)", + "CREATE FUNCTION check_row() RETURNS trigger LANGUAGE plpgsql AS \ + $$ BEGIN RAISE NOTICE 'checked %', NEW.id; RETURN NULL; END $$", + "CREATE CONSTRAINT TRIGGER check_at_commit AFTER INSERT ON checked \ + DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION check_row()", + "CREATE FUNCTION add_row(n int) RETURNS int LANGUAGE sql AS \ + $$ INSERT INTO checked VALUES (n) RETURNING id $$", + ], + ) + .await; + let session = session(connection.as_ref()).await; + + let events = run_ok(session.as_ref(), "SELECT add_row(7)").await; + + assert_eq!(texts(&events), ["7"]); + assert_eq!(notices(&events), ["checked 7"], "{:?}", events.all()); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires docker"] async fn standard_conforming_strings_off_splits_with_backslash_escapes() { diff --git a/core/crates/ffi/src/run/run_event.rs b/core/crates/ffi/src/run/run_event.rs index 4fa2939599..0f392dd79a 100644 --- a/core/crates/ffi/src/run/run_event.rs +++ b/core/crates/ffi/src/run/run_event.rs @@ -42,6 +42,11 @@ pub enum RunEvent { message: String, code: Option, }, + /// The run raised more notices than it keeps; `retained` came before + /// this and the rest are gone. + NoticeLimitReached { + retained: u64, + }, /// The last event of every run, with the transaction the editor's /// connection is left in. Finished { diff --git a/core/crates/ffi/src/run/run_sink.rs b/core/crates/ffi/src/run/run_sink.rs index 711b9b3c68..1234e1d6c7 100644 --- a/core/crates/ffi/src/run/run_sink.rs +++ b/core/crates/ffi/src/run/run_sink.rs @@ -75,6 +75,7 @@ impl ResultSink for RunSink { } ResultEvent::CommandCompleted { rows_affected } => RunEvent::CommandCompleted { rows_affected }, ResultEvent::Notice(notice) => notice.into(), + ResultEvent::NoticeLimitReached { retained } => RunEvent::NoticeLimitReached { retained }, }; self.state.post(event); SinkFlow::Continue @@ -86,3 +87,43 @@ impl RunSink { self.current.as_ref().map_or(u32::MAX, |(index, _)| *index) } } + +#[cfg(test)] +mod tests { + use tablepro_core::ServerNotice; + + use super::*; + use crate::run::RunListener; + + struct Quiet; + + impl RunListener for Quiet { + fn events_pending(&self) {} + } + + #[tokio::test] + async fn the_notice_limit_reaches_the_app_after_the_notices_kept() { + let state = Arc::new(RunState::new(Box::new(Quiet))); + let mut sink = RunSink::new(state.clone()); + + sink.accept(ResultEvent::Notice(ServerNotice { + severity: "NOTICE".to_owned(), + message: "row 1".to_owned(), + code: None, + })) + .await; + sink.accept(ResultEvent::NoticeLimitReached { retained: 1 }).await; + + assert_eq!( + state.drain(), + [ + RunEvent::Notice { + severity: "NOTICE".to_owned(), + message: "row 1".to_owned(), + code: None, + }, + RunEvent::NoticeLimitReached { retained: 1 }, + ] + ); + } +} diff --git a/core/crates/mcp/src/collected_results.rs b/core/crates/mcp/src/collected_results.rs index 7b581e36be..8712a1a366 100644 --- a/core/crates/mcp/src/collected_results.rs +++ b/core/crates/mcp/src/collected_results.rs @@ -107,6 +107,8 @@ impl ResultCollector { state.notices.push(format!("{}: {}", notice.severity, notice.message)); } } + // The answer keeps far fewer notices than a run does. + ResultEvent::NoticeLimitReached { .. } => {} } SinkFlow::Continue } diff --git a/linux/crates/app/src/ui/editor/run_log.rs b/linux/crates/app/src/ui/editor/run_log.rs index 7e0a3dbb55..76167f6e5a 100644 --- a/linux/crates/app/src/ui/editor/run_log.rs +++ b/linux/crates/app/src/ui/editor/run_log.rs @@ -203,6 +203,14 @@ impl BatchCollector { ResultEvent::Notice(notice) => self.entries.push(EntryKind::Notice { message: format!("{}: {}", notice.severity, notice.message), }), + ResultEvent::NoticeLimitReached { retained } => self.entries.push(EntryKind::Notice { + message: crate::i18n::ngettext_f( + "Only the first {n} notice is shown. The server sent more.", + "Only the first {n} notices are shown. The server sent more.", + u32::try_from(retained).unwrap_or(u32::MAX), + &[("n", &retained.to_string())], + ), + }), } } @@ -586,6 +594,47 @@ mod tests { assert!(matches!(entries[2], EntryKind::Command { rows_affected: Some(4) })); } + #[test] + fn notices_past_the_runs_limit_end_in_one_line_that_says_so() { + let notice = |message: &str| { + ResultEvent::Notice(ServerNotice { + severity: "NOTICE".to_owned(), + message: message.to_owned(), + code: None, + }) + }; + + let (entries, _) = collect( + vec![ + notice("row 1"), + notice("row 2"), + ResultEvent::NoticeLimitReached { retained: 2 }, + ResultEvent::CommandCompleted { rows_affected: None }, + ], + RunEnd::Completed, + ); + + let messages: Vec<&str> = entries + .iter() + .filter_map(|entry| match entry { + EntryKind::Notice { message } => Some(message.as_str()), + _ => None, + }) + .collect(); + assert_eq!( + messages, + [ + "NOTICE: row 1", + "NOTICE: row 2", + "Only the first 2 notices are shown. The server sent more." + ] + ); + assert!(matches!( + entries.last(), + Some(EntryKind::Command { rows_affected: None }) + )); + } + #[test] fn a_set_the_driver_cut_at_the_runs_room_names_the_run_limit() { let room = RowRoom { diff --git a/linux/docs/adding-drivers.md b/linux/docs/adding-drivers.md index cc49706c6c..37a394f3fa 100644 --- a/linux/docs/adding-drivers.md +++ b/linux/docs/adding-drivers.md @@ -310,6 +310,7 @@ A run streams `ResultEvent`s into a `ResultSink`: - `ResultSetFinished { retained, total_rows, truncation }` closes the set. `Truncation` is `Complete`, `ServerStopped` or `ReadPastLimit`, and `total_rows` is `None` when the driver did not read the rest. - `CommandCompleted { rows_affected }` reports a statement that returned no rows; `None` is "not reported", not zero. - `Notice` passes on a server notice or warning. +- `NoticeLimitReached { retained }` comes once per run when the driver kept fewer notices than the server sent, after the `retained` ones it passed on. `ResultSink::accept` is awaited before the next rows are read off the wire, so a grid that cannot keep up slows the read instead of growing a queue the size of the result. `SinkFlow::ConsumerGone` ends the run with `RunEnd::Cancelled`. PostgreSQL, MySQL, SQL Server and ClickHouse then stop the statement on the server, SQLite stops stepping it, DuckDB stops fetching its streamed result, and the Cassandra driver asks for no more pages. `stream_read` uses the same sink and events, without a row limit. diff --git a/linux/docs/drivers/postgresql.md b/linux/docs/drivers/postgresql.md index 38c9ca5a06..b8cc744137 100644 --- a/linux/docs/drivers/postgresql.md +++ b/linux/docs/drivers/postgresql.md @@ -103,7 +103,7 @@ An editor tab runs SQL the way psql does: - The script is split on the client with the PostgreSQL grammar and sent one statement per message, stopping at the first failure. When a statement changes `standard_conforming_strings`, the rest of the script is split again with the new rule. - No session-level `SET` is ever sent. The run's deadline and the session's statement timeout are enforced with cancel requests, and nothing new goes to the server until an outstanding cancel request has finished. A statement that has not ended by the end of the cancel grace ends the session. -- Notices (`RAISE NOTICE`, warnings) arrive as `ResultEvent::Notice`. +- Notices (`RAISE NOTICE`, warnings) arrive as `ResultEvent::Notice` with the statement that raised them, those of the cursor's `DECLARE` and its closing `COMMIT` or `RELEASE` included. Those of the driver's own statements, such as its `BEGIN`, `ROLLBACK` and the transaction probe, are dropped. A run keeps 10,000; past that the connection drops the rest and the run sends `NoticeLimitReached` once. - `COPY FROM STDIN` and `COPY TO STDOUT` are refused with `Unsupported`. - Result columns are text: the simple protocol gives a column no type, so every value arrives as `Value::Text`. Typed columns wait on rust-postgres #1358. diff --git a/windows/docs/decisions/0004-app-shell.md b/windows/docs/decisions/0004-app-shell.md index e709beb658..56cb617590 100644 --- a/windows/docs/decisions/0004-app-shell.md +++ b/windows/docs/decisions/0004-app-shell.md @@ -16,6 +16,8 @@ On 2026-11-03 the maintainer settled four more: On 2026-11-04 the roadmap added a page size picker and Go to page to the table pager, as macOS has them, and tab tear-out through TabView's own events. +On 2026-11-05 the maintainer had the app show what the server prints, PostgreSQL's notices and warnings, with the statement that printed them. + ## Decision `src/TablePro.App` is one `MainWindow` with the WinUI `TitleBar`, a `MenuBar` under it and a `NavigationView` in `LeftCompact` whose content is a page object kept per surface: `HomeView`, `SettingsView` and a `WorkspaceView` per open connection. A tab moved out of it goes to a tab window, a `MainWindow` for that one connection (Tab windows, below). The core keeps the saved connections and their secrets; the app keeps its settings and its session in two JSON files in the package's LocalState folder. @@ -111,6 +113,7 @@ On 2026-11-04 the roadmap added a page size picker and Go to page to the table p - Invoking a table or view opens its table tab, or brings the open one forward. The core writes the statement (`DatabaseSession.GetTableQueryAsync`, over the core's `build_page`), quoted, ordered and limited as the engine writes it. - A query tab is the SQL editor over the run's results; a table tab is the results alone. Each opens its own editor connection on first use. - Results: the run's first result set in `DataGridView` (ADR 0003), given to the grid once it has its first rows, or once it ends empty, and grown on each `RowsAppended`. The rows shown stay until then, so a sort or a refresh never shows an empty grid between the two results. Columns are sized to their content, and a sort or a refresh keeps their widths. Copy and Select All in the Edit menu reach the grid when it had focus last. +- Messages: a run the server printed something in gets a `SelectorBar`, Results | Messages, over its results. Messages is a read-only monospace `TextBox` with one line per notice, `{severity}: {message}`, as the Linux app prints the same core's notices, set once per batch of events rather than per notice. A run with no rows opens on Messages, one with rows on Results unless Messages was picked while it ran. Past the 10,000 notices the core keeps per run (`NoticeLimitReached`), the text ends with "The output was cut short." - A header click on a table tab runs the table's query again with the sort: `build_page` writes the ORDER BY, with the row key as the tiebreak, so it orders the whole table, not the rows already read. A row address the engine adds for edits (SQLite's rowid, PostgreSQL's ctid) follows the visible columns and is not shown. An engine that pages in its own order refuses the sort, and the grid keeps the shown one. - A header click on a query result does what the macOS app does: the result's statement runs again with the column's ORDER BY in place of its own and before its row limit (`DatabaseSession.SortedQuery`, the core's `with_order_by`). The ORDER BY names the column, or gives its position where a name is not unique or not a plain column (core ADR 0007). Only one statement that reads runs again. For a script, a write, SHOW or EXPLAIN, and where the engine takes no position, the core sorts the rows the result holds (`ResultSet.SortedAsync`), as macOS sorts them where they are, and the status says so. - The object pane resizes with the Windows Community Toolkit's `GridSplitter` (Sizers), set up as its [page](https://learn.microsoft.com/en-us/dotnet/communitytoolkit/windows/sizers/gridsplitter) lays it out, between 180 and 640 effective pixels; WinUI's `SplitView` has no user resize. Every workspace shares the width, which `session.json` keeps. @@ -175,7 +178,7 @@ Facts the CI runs on `windows-2025` and `windows-11-arm` established, which the - The core gained the library and connecting through it: `Core::library`, `connect_saved`, `test_connection`, `parse_connection_url`, `DatabaseSession::table_query` and `sorted_query` with `sql_syntax::with_order_by` and `sort_reference`, `ResultSet::sorted` over `value::order`, and `Core::with_system_secrets`, which keeps secrets in Credential Manager on Windows (`core/ARCHITECTURE.md`, Platforms). - Then what the shell's second pass needed: `DatabaseDriver::category` (`EngineCategory`) and `DriverInfo.category`; in the library, tags (`create_tag`, `update_tag`, `delete_tag`, `tag_connections`, and the tags in the snapshot), `rename_connection`, `add_favorites` and `remove_favorites` with a place, `move_to_group` with a place and `move_groups`; `ErrorInfo.secret`; `table_query` with an offset and `can_seek`, `table_row_count`; `health`, `health_change` and `reconnect` on the session; `QueryEditor::is_lost`; and the transaction in `RunEvent::Finished`. Each has its test in `core/crates/ffi/tests`. -- `tests/TablePro.App.Model.Tests` covers settings, the session file with the window, the pane and the tab windows, the library tree with tags and search, selection by connection, drops, the engine families, closed tabs, the pager with its page sizes and Go to page, and the workspace bars, the form's fields and the object folders on any machine. `tests/TablePro.App.UITests` drives the unpackaged build through UI Automation (FlaUI): first run, Settings, the form and its engine families, connecting to a SQLite file, opening, sorting and paging a table with a width set through Transform and the header's ItemStatus, a table's page size and Go to page kept across a relaunch, a tab moved to its own window that comes back with its place and merges back by drag, a tab dragged out whose window closes with its last tab, a query and its sort, a script's rows sorted where they are, a failed connect, a password asked for, session restore, the rail's mark, the pane's width and the window's place across a relaunch, Reopen closed tab, an open transaction's bars, and rename, tags, a drop and selection mode on the Connections page, in light and dark, with screenshots. +- `tests/TablePro.App.Model.Tests` covers settings, the session file with the window, the pane and the tab windows, the library tree with tags and search, selection by connection, drops, the engine families, closed tabs, the pager with its page sizes and Go to page, and the workspace bars, a run's messages, the form's fields and the object folders on any machine. `tests/TablePro.App.UITests` drives the unpackaged build through UI Automation (FlaUI): first run, Settings, the form and its engine families, connecting to a SQLite file, opening, sorting and paging a table with a width set through Transform and the header's ItemStatus, a table's page size and Go to page kept across a relaunch, a tab moved to its own window that comes back with its place and merges back by drag, a tab dragged out whose window closes with its last tab, a query and its sort, a script's rows sorted where they are, a failed connect, a password asked for, session restore, the rail's mark, the pane's width and the window's place across a relaunch, Reopen closed tab, an open transaction's bars, a PostgreSQL notice under Messages, and rename, tags, a drop and selection mode on the Connections page, in light and dark, with screenshots. - SSH connections use the core's in-process transport (core ADR 0009). Host-key, passphrase and interactive login prompts use native ContentDialogs. A changed host key requires confirmation to remove its old entry, then separate confirmation of the replacement key on retry. - Not built yet: importing from other apps (the importers move to the core first). - Unverified: Native AOT publishing of the app, and the app on a real Windows 10 machine. diff --git a/windows/src/TablePro.App.Model/RunMessages.cs b/windows/src/TablePro.App.Model/RunMessages.cs new file mode 100644 index 0000000000..10c526923a --- /dev/null +++ b/windows/src/TablePro.App.Model/RunMessages.cs @@ -0,0 +1,43 @@ +using System.Text; +using TablePro.Interop; + +namespace TablePro.App.Model; + +public enum ResultsView +{ + Results, + Messages, +} + +// One line per notice, "{severity}: {message}", as the Linux app shows the same core's notices. +public sealed class RunMessages(string cutShort) +{ + private readonly StringBuilder _text = new(); + + public bool IsEmpty => _text.Length == 0; + + public string Text => _text.ToString(); + + public void Reset() => _text.Clear(); + + public void Add(QueryEvent queryEvent) + { + switch (queryEvent) + { + case QueryEvent.Notice notice: + NewLine().Append(notice.Severity).Append(": ").Append(notice.Message); + break; + case QueryEvent.NoticeLimitReached: + NewLine().Append(cutShort); + break; + default: + break; + } + } + + // A view the user picked while the run went stays; a run that printed and returned no rows opens on what it printed. + public ResultsView ViewAtEnd(bool hasRows, ResultsView? picked) => + IsEmpty ? ResultsView.Results : picked ?? (hasRows ? ResultsView.Results : ResultsView.Messages); + + private StringBuilder NewLine() => _text.Length == 0 ? _text : _text.Append('\n'); +} diff --git a/windows/src/TablePro.App/MainWindow.xaml.cs b/windows/src/TablePro.App/MainWindow.xaml.cs index 7c13e5bd63..541e5e3c0e 100644 --- a/windows/src/TablePro.App/MainWindow.xaml.cs +++ b/windows/src/TablePro.App/MainWindow.xaml.cs @@ -911,6 +911,9 @@ private async void OnCopy(object sender, RoutedEventArgs e) { switch (Workspace?.ActiveTab) { + case { MessagesTarget: { } messages }: + messages.CopySelectionToClipboard(); + break; case { GridIsTarget: true } tab: await tab.Grid.CopySelectionAsync(); break; @@ -932,6 +935,13 @@ private async void OnPaste(object sender, RoutedEventArgs e) private void OnSelectAll(object sender, RoutedEventArgs e) { + if (Workspace?.ActiveTab?.MessagesTarget is { } messages) + { + messages.SelectAll(); + messages.Focus(FocusState.Programmatic); + return; + } + if (Workspace?.ActiveTab is { GridIsTarget: true } tab) { tab.Grid.SelectAll(); diff --git a/windows/src/TablePro.App/Strings/en-US/Resources.resw b/windows/src/TablePro.App/Strings/en-US/Resources.resw index 0371562a75..8fe8639905 100644 --- a/windows/src/TablePro.App/Strings/en-US/Resources.resw +++ b/windows/src/TablePro.App/Strings/en-US/Resources.resw @@ -291,6 +291,18 @@ Results + + Results or messages + + + Results + + + Messages + + + Messages + Connections @@ -747,6 +759,9 @@ The statement returned no rows + + The output was cut short. + Rows: {0:N0} ยท sorted the rows shown diff --git a/windows/src/TablePro.App/Views/ITabContent.cs b/windows/src/TablePro.App/Views/ITabContent.cs index 39961b9fc1..2941c80104 100644 --- a/windows/src/TablePro.App/Views/ITabContent.cs +++ b/windows/src/TablePro.App/Views/ITabContent.cs @@ -1,3 +1,4 @@ +using Microsoft.UI.Xaml.Controls; using TablePro.App.Model; using TablePro.DataGrid; using TablePro.Editor; @@ -23,6 +24,9 @@ internal interface ITabContent // Copy and Select All reach the grid when it, not the editor, had focus last. bool GridIsTarget { get; } + // Copy and Select All reach the run's messages while they are shown and had focus last. + TextBox? MessagesTarget { get; } + void Run(); void Stop(); diff --git a/windows/src/TablePro.App/Views/QueryTabView.xaml.cs b/windows/src/TablePro.App/Views/QueryTabView.xaml.cs index de40409201..5292063b21 100644 --- a/windows/src/TablePro.App/Views/QueryTabView.xaml.cs +++ b/windows/src/TablePro.App/Views/QueryTabView.xaml.cs @@ -22,6 +22,7 @@ public sealed partial class QueryTabView : UserControl, ITabContent private string? _statement; private GridSort _sort = GridSort.None; private bool _sorting; + private EditTarget _target; internal QueryTabView(Workbench workbench, SessionShare share, string title, string? sql) { @@ -36,8 +37,9 @@ internal QueryTabView(Workbench workbench, SessionShare share, string title, str AutomationProperties.SetName(SqlText, Strings.Format("EditorName", title)); Results.StatusChanged += (_, status) => StatusText.Text = status; Results.SortRequested += OnSortRequested; - SqlText.GotFocus += (_, _) => GridIsTarget = false; - Results.GridView.GotFocus += (_, _) => GridIsTarget = true; + SqlText.GotFocus += (_, _) => _target = EditTarget.Editor; + Results.GridView.GotFocus += (_, _) => _target = EditTarget.Grid; + Results.MessagesBox.GotFocus += (_, _) => _target = EditTarget.Messages; } public event EventHandler? StateChanged; @@ -52,7 +54,10 @@ internal QueryTabView(Workbench workbench, SessionShare share, string title, str public DataGridView Grid => Results.GridView; - public bool GridIsTarget { get; private set; } + public bool GridIsTarget => _target == EditTarget.Grid; + + public TextBox? MessagesTarget => + _target == EditTarget.Messages && Results.MessagesBox.Visibility == Visibility.Visible ? Results.MessagesBox : null; public async void Run() => await RunAsync().ConfigureAwait(true); @@ -293,4 +298,11 @@ private async Task SortHeldRowsAsync(GridSort sort) private void OnRun(object sender, RoutedEventArgs e) => Run(); private void OnStop(object sender, RoutedEventArgs e) => Stop(); + + private enum EditTarget + { + Editor, + Grid, + Messages, + } } diff --git a/windows/src/TablePro.App/Views/ResultsPane.xaml b/windows/src/TablePro.App/Views/ResultsPane.xaml index c465cbd9a5..80d942ddb6 100644 --- a/windows/src/TablePro.App/Views/ResultsPane.xaml +++ b/windows/src/TablePro.App/Views/ResultsPane.xaml @@ -7,6 +7,7 @@ + @@ -19,23 +20,57 @@ IsOpen="False" Severity="Error" /> - + + Margin="4,0" + AutomationProperties.AutomationId="ResultsViews" + SelectionChanged="OnViewChanged" + Visibility="Collapsed"> + + + - + + + + + + + diff --git a/windows/src/TablePro.App/Views/ResultsPane.xaml.cs b/windows/src/TablePro.App/Views/ResultsPane.xaml.cs index d29aff98af..a74710b558 100644 --- a/windows/src/TablePro.App/Views/ResultsPane.xaml.cs +++ b/windows/src/TablePro.App/Views/ResultsPane.xaml.cs @@ -1,5 +1,6 @@ using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; +using TablePro.App.Model; using TablePro.App.Services; using TablePro.DataGrid; using TablePro.DataGrid.Model; @@ -10,8 +11,12 @@ namespace TablePro.App.Views; // A sort of the rows held shows a sorted copy, so the run's own set keeps arrival order for the next sort. public sealed partial class ResultsPane : UserControl { + private readonly RunMessages _messages = new(Strings.Get("MessagesCutShort")); private ResultSet? _source; private ResultSet? _shown; + private ResultsView _view; + private ResultsView? _picked; + private bool _messagesQueued; public ResultsPane() { @@ -30,6 +35,8 @@ public ResultsPane() public DataGridView GridView => ResultGrid; + public TextBox MessagesBox => MessagesText; + public IReadOnlyList Columns { get; private set; } = []; // Only a run with one result set can be sorted by running its statement again. @@ -92,6 +99,9 @@ public void Clear() Release(); ErrorBar.IsOpen = false; Placeholder.Text = Strings.Get("ResultsEmpty"); + _messages.Reset(); + ShowMessages(); + SelectView(ResultsView.Results); } // The rows shown stay until the new set has rows or ends, so a sort or a refresh never blanks the grid. @@ -101,6 +111,9 @@ internal async Task ShowRunAsync(QueryRun run, GridSort sort, int visibleColumns ErrorBar.IsOpen = false; Status(Strings.Get("StatusRunning")); ResultSetCount = 0; + // The last run's messages stay on screen until this one's arrive or it ends, as its rows do. + _messages.Reset(); + _picked = null; var shownIndex = -1; (ResultSet Set, IReadOnlyList Columns)? waiting = null; long rows = 0; @@ -112,6 +125,10 @@ void ShowWaiting() { waiting = null; Show(next.Set, next.Columns, sort); + if (_picked is null) + { + SelectView(ResultsView.Results); + } } } @@ -147,11 +164,17 @@ void ShowWaiting() case QueryEvent.CommandCompleted command: affected = (affected ?? 0) + (command.RowsAffected ?? 0); break; + case QueryEvent.Notice or QueryEvent.NoticeLimitReached: + _messages.Add(queryEvent); + QueueMessages(); + break; case QueryEvent.Finished finished: ShowWaiting(); RowCount = shownIndex >= 0 ? rows : 0; Transaction = finished.Transaction; Finish(finished.Outcome, shownIndex >= 0, rows, affected, truncated); + ShowMessages(); + SelectView(_messages.ViewAtEnd(shownIndex >= 0, _picked)); break; default: break; @@ -238,5 +261,42 @@ private void Finish(QueryOutcome outcome, bool hasRows, long rows, long? affecte } } + // Notices that arrive together are laid out once, after the batch: a TextBox lays out all its text on each change. + private void QueueMessages() + { + if (!_messagesQueued) + { + _messagesQueued = DispatcherQueue.TryEnqueue(ShowMessages); + } + } + + private void ShowMessages() + { + _messagesQueued = false; + MessagesText.Text = _messages.Text; + ViewBar.Visibility = _messages.IsEmpty ? Visibility.Collapsed : Visibility.Visible; + } + + private void SelectView(ResultsView view) + { + _view = view; + ViewBar.SelectedItem = view == ResultsView.Messages ? MessagesItem : ResultsItem; + ResultsArea.Visibility = view == ResultsView.Results ? Visibility.Visible : Visibility.Collapsed; + MessagesText.Visibility = view == ResultsView.Messages ? Visibility.Visible : Visibility.Collapsed; + } + + private void OnViewChanged(SelectorBar sender, SelectorBarSelectionChangedEventArgs args) + { + var view = ReferenceEquals(sender.SelectedItem, MessagesItem) ? ResultsView.Messages : ResultsView.Results; + // SelectView's own change comes back here with the view it already set. + if (view == _view) + { + return; + } + + _picked = view; + SelectView(view); + } + private void Status(string text) => StatusChanged?.Invoke(this, text); } diff --git a/windows/src/TablePro.App/Views/TableTabView.xaml.cs b/windows/src/TablePro.App/Views/TableTabView.xaml.cs index a13ad4df44..b84a79d446 100644 --- a/windows/src/TablePro.App/Views/TableTabView.xaml.cs +++ b/windows/src/TablePro.App/Views/TableTabView.xaml.cs @@ -67,6 +67,8 @@ internal TableTabView(Workbench workbench, SessionShare share, string? schema, s public bool GridIsTarget => true; + public TextBox? MessagesTarget => null; + private int PageSize => _chosenSize ?? _workbench.Settings.PageSize; public bool Shows(string? schema, string table) => diff --git a/windows/src/TablePro.Interop/Generated/tablepro_ffi.cs b/windows/src/TablePro.Interop/Generated/tablepro_ffi.cs index e304d827b2..e79c146e4f 100644 --- a/windows/src/TablePro.Interop/Generated/tablepro_ffi.cs +++ b/windows/src/TablePro.Interop/Generated/tablepro_ffi.cs @@ -7965,6 +7965,14 @@ public record Notice ( string? Code ) : RunEvent {} + /// + /// The run raised more notices than it keeps; `retained` came before + /// this and the rest are gone. + /// + public record NoticeLimitReached ( + ulong Retained + ) : RunEvent {} + /// /// The last event of every run, with the transaction the editor's /// connection is left in. @@ -8022,6 +8030,10 @@ public override RunEvent Read(BigEndianStream stream) { FfiConverterOptionalString.INSTANCE.Read(stream) ); case 8: + return new RunEvent.NoticeLimitReached( + FfiConverterUInt64.INSTANCE.Read(stream) + ); + case 9: return new RunEvent.Finished( FfiConverterTypeRunOutcome.INSTANCE.Read(stream), FfiConverterTypeTransactionStateTag.INSTANCE.Read(stream) @@ -8063,6 +8075,9 @@ public override int AllocationSize(RunEvent value) { + FfiConverterString.INSTANCE.AllocationSize(variant_value.Severity) + FfiConverterString.INSTANCE.AllocationSize(variant_value.Message) + FfiConverterOptionalString.INSTANCE.AllocationSize(variant_value.Code); + case RunEvent.NoticeLimitReached variant_value: + return 4 + + FfiConverterUInt64.INSTANCE.AllocationSize(variant_value.Retained); case RunEvent.Finished variant_value: return 4 + FfiConverterTypeRunOutcome.INSTANCE.AllocationSize(variant_value.Outcome) @@ -8111,8 +8126,12 @@ public override void Write(RunEvent value, BigEndianStream stream) { FfiConverterString.INSTANCE.Write(variant_value.Message, stream); FfiConverterOptionalString.INSTANCE.Write(variant_value.Code, stream); break; - case RunEvent.Finished variant_value: + case RunEvent.NoticeLimitReached variant_value: stream.WriteInt(8); + FfiConverterUInt64.INSTANCE.Write(variant_value.Retained, stream); + break; + case RunEvent.Finished variant_value: + stream.WriteInt(9); FfiConverterTypeRunOutcome.INSTANCE.Write(variant_value.Outcome, stream); FfiConverterTypeTransactionStateTag.INSTANCE.Write(variant_value.Transaction, stream); break; diff --git a/windows/src/TablePro.Interop/Mapping.cs b/windows/src/TablePro.Interop/Mapping.cs index b3fed7419c..98f65f81a7 100644 --- a/windows/src/TablePro.Interop/Mapping.cs +++ b/windows/src/TablePro.Interop/Mapping.cs @@ -123,6 +123,7 @@ public static ResultColumn ResultColumn(Native.ResultColumnInfo column) => Native.RunEvent.CommandCompleted command => new QueryEvent.CommandCompleted(command.RowsAffected is { } affected ? Count(affected) : null), Native.RunEvent.Notice notice => new QueryEvent.Notice(notice.Severity, notice.Message, notice.Code), + Native.RunEvent.NoticeLimitReached limit => new QueryEvent.NoticeLimitReached(Count(limit.Retained)), Native.RunEvent.Finished finished => new QueryEvent.Finished(Outcome(finished.Outcome), Transaction(finished.Transaction)), _ => throw new InvalidOperationException($"The core sent an event this build does not know: {nativeEvent}"), diff --git a/windows/src/TablePro.Interop/QueryEvent.cs b/windows/src/TablePro.Interop/QueryEvent.cs index be158d122f..63d498e8ca 100644 --- a/windows/src/TablePro.Interop/QueryEvent.cs +++ b/windows/src/TablePro.Interop/QueryEvent.cs @@ -42,6 +42,9 @@ public sealed record CommandCompleted(long? RowsAffected) : QueryEvent; // Something the server said that is not an error. public sealed record Notice(string Severity, string Message, string? Code) : QueryEvent; + // The run raised more notices than it keeps: Retained came before this, the rest are gone. + public sealed record NoticeLimitReached(long Retained) : QueryEvent; + // Transaction: the state the editor's connection is left in. public sealed record Finished(QueryOutcome Outcome, TransactionState Transaction = TransactionState.Unknown) : QueryEvent; diff --git a/windows/tests/TablePro.App.Model.Tests/RunMessagesTests.cs b/windows/tests/TablePro.App.Model.Tests/RunMessagesTests.cs new file mode 100644 index 0000000000..3b33f9f426 --- /dev/null +++ b/windows/tests/TablePro.App.Model.Tests/RunMessagesTests.cs @@ -0,0 +1,64 @@ +using TablePro.Interop; + +namespace TablePro.App.Model.Tests; + +public sealed class RunMessagesTests +{ + private const string CutShort = "The output was cut short."; + + [Fact] + public void EachNoticeIsALineWithItsSeverity() + { + var messages = new RunMessages(CutShort); + + messages.Add(new QueryEvent.Notice("NOTICE", "step 1 done", "00000")); + messages.Add(new QueryEvent.CommandCompleted(null)); + messages.Add(new QueryEvent.Notice("WARNING", "w", "01000")); + + Assert.Equal("NOTICE: step 1 done\nWARNING: w", messages.Text); + } + + [Fact] + public void TheNoticeLimitSaysTheOutputWasCutShort() + { + var messages = new RunMessages(CutShort); + + messages.Add(new QueryEvent.Notice("NOTICE", "row 1", null)); + messages.Add(new QueryEvent.NoticeLimitReached(1)); + + Assert.Equal("NOTICE: row 1\nThe output was cut short.", messages.Text); + } + + [Fact] + public void ANewRunStartsEmpty() + { + var messages = new RunMessages(CutShort); + messages.Add(new QueryEvent.Notice("NOTICE", "earlier", null)); + + messages.Reset(); + + Assert.True(messages.IsEmpty); + Assert.Equal(string.Empty, messages.Text); + } + + [Fact] + public void ARunWithNoRowsOpensOnWhatItPrinted() + { + var messages = new RunMessages(CutShort); + messages.Add(new QueryEvent.Notice("NOTICE", "hi", null)); + + Assert.Equal(ResultsView.Messages, messages.ViewAtEnd(hasRows: false, picked: null)); + Assert.Equal(ResultsView.Results, messages.ViewAtEnd(hasRows: true, picked: null)); + Assert.Equal(ResultsView.Results, messages.ViewAtEnd(hasRows: false, picked: ResultsView.Results)); + Assert.Equal(ResultsView.Messages, messages.ViewAtEnd(hasRows: true, picked: ResultsView.Messages)); + } + + [Fact] + public void ARunThatPrintedNothingShowsItsResults() + { + var messages = new RunMessages(CutShort); + + Assert.Equal(ResultsView.Results, messages.ViewAtEnd(hasRows: false, picked: null)); + Assert.Equal(ResultsView.Results, messages.ViewAtEnd(hasRows: false, picked: ResultsView.Messages)); + } +} diff --git a/windows/tests/TablePro.App.UITests/NoticeTests.cs b/windows/tests/TablePro.App.UITests/NoticeTests.cs new file mode 100644 index 0000000000..36a509105e --- /dev/null +++ b/windows/tests/TablePro.App.UITests/NoticeTests.cs @@ -0,0 +1,60 @@ +using System.Globalization; +using FlaUI.Core.AutomationElements; +using FlaUI.Core.Tools; +using FlaUI.Core.WindowsAPI; +using TablePro.Testing; + +namespace TablePro.App.UITests; + +public sealed partial class ShellTests +{ + [Theory] + [MemberData(nameof(Themes))] + public void APostgresNoticeShowsUnderMessages(string theme) + { + Assert.SkipUnless(SshTestEnvironment.Available, "Requires the Windows PostgreSQL fixture."); + using var app = AppSession.Launch(theme); + app.Click(app.Find("NewConnectionCard"), () => app.HasWindow("New connection")); + var form = app.WaitForWindow("New connection"); + app.Click(app.Find("Engine_postgres", form), () => app.TryFind("HostBox", form) is not null, form); + app.Find("NameBox", form).AsTextBox().Text = "PostgreSQL notices"; + app.Find("HostBox", form).AsTextBox().Text = "127.0.0.1"; + app.Find("PortBox", form).AsTextBox().Text = SshTestEnvironment.DatabasePort.ToString(CultureInfo.InvariantCulture); + app.Find("UserBox", form).AsTextBox().Text = "tablepro_ssh"; + app.Find("DatabaseBox", form).AsTextBox().Text = "tablepro_ssh"; + app.Find("PasswordInput", form).Patterns.Value.Pattern.SetValue(SshTestEnvironment.DatabasePassword); + app.Find("ConnectButton", form).AsButton().Invoke(); + app.WaitForClose("New connection"); + app.FindByName("ssh_proof", app.Find("ObjectTree")); + + // No rows, so the run opens on what it printed. + app.TypeInto("Query 1", "DO $$ BEGIN RAISE NOTICE 'hello from TablePro'; END $$"); + app.Chord(MessagesShown, VirtualKeyShort.CONTROL, VirtualKeyShort.ENTER); + Assert.Contains("NOTICE: hello from TablePro", WaitForMessages("hello from TablePro"), StringComparison.Ordinal); + app.Screenshot("37-postgres-messages"); + + // Rows open on the grid, and Messages holds what the script printed before them. + app.TypeInto("Query 1", "DO $$ BEGIN RAISE NOTICE 'before the rows'; END $$; SELECT answer FROM ssh_proof"); + app.Chord( + () => app.Find("QueryStatus").Properties.Name.ValueOrDefault?.StartsWith("Rows: 1", StringComparison.Ordinal) == true, + VirtualKeyShort.CONTROL, + VirtualKeyShort.ENTER); + Assert.Equal("42", app.WaitForCell(app.Find("ResultsGrid"), 0, 0, value => value == "42")); + Assert.False(MessagesShown()); + app.Click(app.Find("MessagesViewItem"), MessagesShown); + Assert.Contains("NOTICE: before the rows", WaitForMessages("before the rows"), StringComparison.Ordinal); + app.Screenshot("38-postgres-messages-beside-rows"); + + bool MessagesShown() => app.TryFind("ResultsMessages") is { IsOffscreen: false }; + + string WaitForMessages(string expected) + { + var text = string.Empty; + Retry.WhileFalse( + () => (text = app.Find("ResultsMessages").AsTextBox().Text).Contains(expected, StringComparison.Ordinal), + AppSession.Wait, + throwOnTimeout: false); + return text; + } + } +} diff --git a/windows/tests/TablePro.Interop.Tests/NoticeTests.cs b/windows/tests/TablePro.Interop.Tests/NoticeTests.cs new file mode 100644 index 0000000000..0a60eaf277 --- /dev/null +++ b/windows/tests/TablePro.Interop.Tests/NoticeTests.cs @@ -0,0 +1,67 @@ +using TablePro.Testing; +using Native = TablePro.Interop.Native; + +namespace TablePro.Interop.Tests; + +public sealed class NoticeTests : IDisposable +{ + private readonly string _directory = Directory.CreateTempSubdirectory("tablepro-notices-").FullName; + private readonly TableProCore _core; + + public NoticeTests() + { + _core = TableProCore.Create(new HostConfiguration("notice-test", _directory), new MemorySecretStore()); + } + + [Fact] + public void ANoticeKeepsItsSeverityMessageAndCode() + { + var mapped = Mapping.Event(new Native.RunEvent.Notice("WARNING", "w", "01000")); + + Assert.Equal(new QueryEvent.Notice("WARNING", "w", "01000"), mapped); + } + + [Fact] + public void TheNoticeLimitKeepsHowManyCameBeforeIt() + { + var mapped = Mapping.Event(new Native.RunEvent.NoticeLimitReached(10_000)); + + Assert.Equal(new QueryEvent.NoticeLimitReached(10_000), mapped); + } + + [Fact] + public async Task APostgresRunPastTheNoticeLimitEndsItsNoticesWithTheLimit() + { + Assert.SkipUnless(SshTestEnvironment.Available, "Requires the Windows PostgreSQL fixture."); + var cancel = TestContext.Current.CancellationToken; + var saved = await _core.Library.SaveAsync( + new SavedConnection(null, "PostgreSQL", "postgres", "127.0.0.1", SshTestEnvironment.DatabasePort, + "tablepro_ssh", "tablepro_ssh", TlsSettings.Disabled), + new ConnectionSecrets(DatabasePassword: SshTestEnvironment.DatabasePassword)); + await using var session = await _core.ConnectAsync(saved.Id!.Value, cancellationToken: cancel); + await using var editor = await session.OpenEditorAsync(cancel); + + using var run = editor.Run( + "DO $$ BEGIN FOR i IN 1..10005 LOOP RAISE NOTICE 'row %', i; END LOOP; END $$", + cancellationToken: cancel); + var events = new List(); + await foreach (var queryEvent in run.ReadEventsAsync(cancel)) + { + events.Add(queryEvent); + } + + Assert.IsType(await run.Completion); + var notices = events.OfType().ToList(); + Assert.Equal(10_000, notices.Count); + Assert.Equal(("row 1", "00000"), (notices[0].Message, notices[0].Code)); + var limit = Assert.Single(events.OfType()); + Assert.Equal(10_000, limit.Retained); + Assert.True(events.IndexOf(limit) > events.IndexOf(notices[^1])); + } + + public void Dispose() + { + _core.Dispose(); + Directory.Delete(_directory, recursive: true); + } +} From 4fc8f4ab4601f90973a6a4268c67f0b4820d59a2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 18:56:48 +0700 Subject: [PATCH 2/4] fix(windows): route Copy and Select All to a table tab's messages --- windows/src/TablePro.App/Views/TableTabView.xaml.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/windows/src/TablePro.App/Views/TableTabView.xaml.cs b/windows/src/TablePro.App/Views/TableTabView.xaml.cs index b84a79d446..39f90c89a5 100644 --- a/windows/src/TablePro.App/Views/TableTabView.xaml.cs +++ b/windows/src/TablePro.App/Views/TableTabView.xaml.cs @@ -27,6 +27,7 @@ public sealed partial class TableTabView : UserControl, ITabContent // The size this tab was given, as macOS keeps one per table tab; without one, Settings decides. private int? _chosenSize; + private bool _messagesIsTarget; private (long Milliseconds, bool Truncated)? _finished; private CancellationTokenSource? _counting; @@ -49,6 +50,8 @@ internal TableTabView(Workbench workbench, SessionShare share, string? schema, s return RowsStatus()!; }; Results.SortRequested += OnSortRequested; + Results.GridView.GotFocus += (_, _) => _messagesIsTarget = false; + Results.MessagesBox.GotFocus += (_, _) => _messagesIsTarget = true; UpdatePager(); } @@ -67,7 +70,8 @@ internal TableTabView(Workbench workbench, SessionShare share, string? schema, s public bool GridIsTarget => true; - public TextBox? MessagesTarget => null; + public TextBox? MessagesTarget => + _messagesIsTarget && Results.MessagesBox.Visibility == Visibility.Visible ? Results.MessagesBox : null; private int PageSize => _chosenSize ?? _workbench.Settings.PageSize; From e35d30dbfb844edc7dbf6fef2ecc5187c925cf4e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 19:03:48 +0700 Subject: [PATCH 3/4] docs(changelog): add fragments for #4061 --- apple/changelog.d/4061.added.md | 1 + apple/changelog.d/4061.fixed.md | 1 + linux/changelog.d/4061.fixed.1.md | 1 + linux/changelog.d/4061.fixed.md | 1 + 4 files changed, 4 insertions(+) create mode 100644 apple/changelog.d/4061.added.md create mode 100644 apple/changelog.d/4061.fixed.md create mode 100644 linux/changelog.d/4061.fixed.1.md create mode 100644 linux/changelog.d/4061.fixed.md diff --git a/apple/changelog.d/4061.added.md b/apple/changelog.d/4061.added.md new file mode 100644 index 0000000000..42e36b811c --- /dev/null +++ b/apple/changelog.d/4061.added.md @@ -0,0 +1 @@ +PostgreSQL notices such as RAISE NOTICE show in the Output view, and a plan from EXPLAIN ANALYZE gains Output too. diff --git a/apple/changelog.d/4061.fixed.md b/apple/changelog.d/4061.fixed.md new file mode 100644 index 0000000000..86a163dc2a --- /dev/null +++ b/apple/changelog.d/4061.fixed.md @@ -0,0 +1 @@ +Oracle output from a grid save, Commit or an AI statement showed under the next statement run from the editor. diff --git a/linux/changelog.d/4061.fixed.1.md b/linux/changelog.d/4061.fixed.1.md new file mode 100644 index 0000000000..b2add511fc --- /dev/null +++ b/linux/changelog.d/4061.fixed.1.md @@ -0,0 +1 @@ +A row-limited PostgreSQL query lost the warnings the server raised while planning it. diff --git a/linux/changelog.d/4061.fixed.md b/linux/changelog.d/4061.fixed.md new file mode 100644 index 0000000000..e44893433b --- /dev/null +++ b/linux/changelog.d/4061.fixed.md @@ -0,0 +1 @@ +A PostgreSQL statement that raised millions of notices kept every one of them in memory. From d8ceeaad4184f44baeb8a0a70002916caaef7d3a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 6 Nov 2026 09:50:32 +0700 Subject: [PATCH 4/4] fix(windows): drop the stray URL fields from FollowTlsDefault --- windows/src/TablePro.App.Model/ConnectionDraft.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/windows/src/TablePro.App.Model/ConnectionDraft.cs b/windows/src/TablePro.App.Model/ConnectionDraft.cs index 17e606092b..12417cda20 100644 --- a/windows/src/TablePro.App.Model/ConnectionDraft.cs +++ b/windows/src/TablePro.App.Model/ConnectionDraft.cs @@ -198,11 +198,6 @@ public void FollowTlsDefault(DatabaseEngine engine) TlsMode = engine.NewConnectionTls(ParsePort(Port)); TlsModeChosen = false; } - - CaFile = url.CaFile ?? CaFile; - ClientCertificate = url.ClientCertificate ?? ClientCertificate; - ClientKey = url.ClientKey ?? ClientKey; - ServerName = url.ServerName ?? ServerName; } public IReadOnlyList Validate()