From 49fb9e13cb496c8c5c38a597ff5ff1b23e867536 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Thu, 23 Jul 2026 14:41:13 +0800 Subject: [PATCH] fix(query): surface PostgreSQL batch errors --- .../__tests__/useSqlExecution.spec.ts | 31 +++++++++++++++++++ .../src/composables/useSqlExecution.ts | 7 ++--- .../__tests__/query/queryResultError.spec.ts | 16 +++++++++- .../desktop/src/lib/query/queryResultError.ts | 6 ++++ .../queryStore.multiStatementError.spec.ts | 30 ++++++++++++++++++ apps/desktop/src/stores/queryStore.ts | 4 +-- 6 files changed, 87 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts b/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts index 69deea4345..08df6d4b1e 100644 --- a/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts +++ b/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts @@ -243,6 +243,37 @@ describe("useSqlExecution", () => { expect(refreshObjects).not.toHaveBeenCalled(); }); + it("records a later PostgreSQL batch error and skips metadata refresh", async () => { + const activeTab = ref(queryTab("app")); + const activeConnection = ref(connection("postgres")); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + const historyStore = useHistoryStore(); + const connectionStore = useConnectionStore(); + vi.spyOn(queryStore, "executeCurrentSql").mockImplementation(async () => { + const tab = activeTab.value; + if (!tab) return; + const successfulResult = { columns: ["value"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }; + tab.result = successfulResult; + tab.results = [successfulResult, { columns: ["Error"], execution_error: true, rows: [["relation missing_table does not exist"]], affected_rows: 0, execution_time_ms: 1 }]; + tab.activeResultIndex = 0; + }); + const addHistory = vi.spyOn(historyStore, "add").mockResolvedValue(undefined); + const refreshObjects = vi.spyOn(connectionStore, "refreshObjectListTreeNode").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => activeTab.value), + activeConnection: computed(() => activeConnection.value), + executableSql: computed(() => "BEGIN; SELECT 1 AS value; SELECT * FROM missing_table"), + activeOutputView, + }); + + await execution.tryExecute(); + + expect(addHistory).toHaveBeenCalledWith(expect.objectContaining({ success: false, error: "relation missing_table does not exist", affected_rows: undefined })); + expect(refreshObjects).not.toHaveBeenCalled(); + }); + it("does not treat an unmarked MySQL Error alias as a batch failure", async () => { const activeTab = ref(queryTab("app")); const activeConnection = ref(connection("mysql")); diff --git a/apps/desktop/src/composables/useSqlExecution.ts b/apps/desktop/src/composables/useSqlExecution.ts index 42de97b4da..72377aca57 100644 --- a/apps/desktop/src/composables/useSqlExecution.ts +++ b/apps/desktop/src/composables/useSqlExecution.ts @@ -9,7 +9,7 @@ import { isSingleDatabase, usesTreeSchemaMode } from "@/lib/database/databaseCap import { supportsConnectionLevelSqlExecution } from "@/lib/connection/connectionLevelDatabaseBootstrap"; import { classifySqlActivityKind } from "@/lib/history/historyActivityKind"; import { sqlMetadataRefreshTarget } from "@/lib/sql/sqlMetadataRefresh"; -import { isMysqlExecutionErrorResult, usesMysqlProtocolDatabaseType } from "@/lib/query/queryResultError"; +import { isQueryExecutionErrorResult, usesMysqlProtocolDatabaseType } from "@/lib/query/queryResultError"; import { classifyRedisCommandSafety, firstRedisCommandToken } from "@/lib/redis/redisCommandSafety"; import { isSqlExecutionSnapshot, resolveExecutableSql, type SqlExecutionOverride, type SqlExecutionSnapshot } from "@/lib/sql/sqlExecutionTarget"; import { isElasticsearchRestRequestText, parseElasticsearchRestRequestTarget, splitSqlStatementRanges } from "@/lib/sql/sqlStatementRanges"; @@ -61,12 +61,11 @@ function primarySqlOperation(sql: string): string { function firstQueryExecutionError(tab: Pick, databaseType: DatabaseType | undefined) { const activeResult = tab.result; - if (activeResult && isMysqlExecutionErrorResult(activeResult, databaseType)) return activeResult; + if (activeResult && isQueryExecutionErrorResult(activeResult)) return activeResult; if (!usesMysqlProtocolDatabaseType(databaseType) && activeResult?.columns.includes("Error")) return activeResult; - if (!usesMysqlProtocolDatabaseType(databaseType)) return undefined; const results = tab.results?.length ? tab.results : tab.result ? [tab.result] : []; - return results.find((result) => isMysqlExecutionErrorResult(result, databaseType)); + return results.find((result) => isQueryExecutionErrorResult(result)); } export function useSqlExecution(deps: { diff --git a/apps/desktop/src/lib/__tests__/query/queryResultError.spec.ts b/apps/desktop/src/lib/__tests__/query/queryResultError.spec.ts index 4d848da3bb..a7497ff06f 100644 --- a/apps/desktop/src/lib/__tests__/query/queryResultError.spec.ts +++ b/apps/desktop/src/lib/__tests__/query/queryResultError.spec.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import type { QueryResult } from "@/types/database"; -import { isMysqlExecutionErrorResult, isNoSnapshotErrorResult } from "@/lib/query/queryResultError"; +import { isMysqlExecutionErrorResult, isNoSnapshotErrorResult, isQueryExecutionErrorResult } from "@/lib/query/queryResultError"; function errorResult(message: string): QueryResult { return { columns: ["Error"], rows: [[message]], affected_rows: 0, execution_time_ms: 0 }; @@ -59,3 +59,17 @@ describe("isMysqlExecutionErrorResult", () => { expect(isMysqlExecutionErrorResult({ ...errorResult("Duplicate entry '1'"), execution_error: true }, "jdbc")).toBe(false); }); }); + +describe("isQueryExecutionErrorResult", () => { + it("recognizes explicit execution errors for PostgreSQL", () => { + expect(isQueryExecutionErrorResult({ ...errorResult("relation does not exist"), execution_error: true })).toBe(true); + }); + + it("requires an explicit marker for PostgreSQL result groups", () => { + expect(isQueryExecutionErrorResult(errorResult("relation does not exist"))).toBe(false); + }); + + it("does not treat a successful MySQL Error alias as a failure", () => { + expect(isQueryExecutionErrorResult({ ...dataResult(["Error"]), rows: [["2"]] })).toBe(false); + }); +}); diff --git a/apps/desktop/src/lib/query/queryResultError.ts b/apps/desktop/src/lib/query/queryResultError.ts index 43d6a4ad09..fda218c2ba 100644 --- a/apps/desktop/src/lib/query/queryResultError.ts +++ b/apps/desktop/src/lib/query/queryResultError.ts @@ -18,6 +18,12 @@ export function isMysqlExecutionErrorResult(result: QueryResult, databaseType: D return usesMysqlProtocolDatabaseType(databaseType) && result.execution_error === true; } +export function isQueryExecutionErrorResult(result: QueryResult): boolean { + // Multi-database batch responses use this explicit marker so a successful + // query column named Error is never promoted as the failed statement. + return result.execution_error === true; +} + export function isNoSnapshotErrorResult(result: QueryResult | undefined | null): boolean { if (!result || !result.columns.includes("Error") || result.rows.length === 0) return false; return NO_SNAPSHOT_ERROR_PATTERN.test(String(result.rows[0]?.[0] ?? "")); diff --git a/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts index a8370e7d28..533dbdea70 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts @@ -83,6 +83,36 @@ describe("queryStore multi-statement errors", () => { expect(tab.result?.columns).toEqual(["Error"]); }); + it("opens a later PostgreSQL error result from a mixed result batch", async () => { + mocks.getConnectionConfig.mockReturnValue({ + id: "postgres-1", + name: "PostgreSQL", + db_type: "postgres", + database: "app", + query_timeout_secs: 30, + }); + mocks.executeMulti.mockResolvedValue([ + { columns: [], rows: [], affected_rows: 0, execution_time_ms: 1, statement_index: 0 }, + { columns: ["value"], rows: [[1]], affected_rows: 0, execution_time_ms: 1, statement_index: 1 }, + { columns: ["Error"], execution_error: true, rows: [["relation missing_table does not exist"]], affected_rows: 0, execution_time_ms: 1, statement_index: 2 }, + { columns: ["after_error"], rows: [[2]], affected_rows: 0, execution_time_ms: 1, statement_index: 3 }, + ]); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("postgres-1", "app", "Query"); + + await store.executeTabSql(tabId, "BEGIN; SELECT 1 AS value; SELECT * FROM missing_table"); + + const tab = store.tabs.find((item) => item.id === tabId)!; + expect(tab.activeResultIndex).toBe(2); + expect(tab.result).toMatchObject({ + columns: ["Error"], + execution_error: true, + rows: [["relation missing_table does not exist"]], + }); + expect(tab.results).toHaveLength(4); + }); + it("invalidates only the executing Oracle tab after successful CURRENT_SCHEMA changes", async () => { mocks.getConnectionConfig.mockReturnValue({ id: "oracle-1", diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index d5a4274662..cfc4742b05 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -49,7 +49,7 @@ import { clearDataGridPendingSnapshotsForTab } from "@/composables/useDataGridEd import { buildTabResultSnapshot, deleteTabResultSnapshot, pruneTabResultSnapshots, readTabResultSnapshot, tabResultCacheKey, writeTabResultSnapshot } from "@/lib/tabs/tabResultCache"; import { estimateQueryResultsBytes, selectInactiveResultEvictions } from "@/lib/tabs/queryResultSize"; import { queryResultBaseSql, queryResultExecutionSql } from "@/lib/tabs/tabPresentation"; -import { isMysqlExecutionErrorResult } from "@/lib/query/queryResultError"; +import { isQueryExecutionErrorResult } from "@/lib/query/queryResultError"; import { decodeQueryResultArchive, encodeQueryResultArchive, type DecodedQueryResultArchive } from "@/lib/query/queryResultArchive"; import * as api from "@/lib/backend/api"; import { useConnectionStore } from "@/stores/connectionStore"; @@ -3472,7 +3472,7 @@ export const useQueryStore = defineStore("query", () => { current.results[activeGroupIndex] = results[0]; current.result = results[0]; } else if (results.length > 1) { - const errorResultIndex = results.findIndex((result) => isMysqlExecutionErrorResult(result, conn?.db_type)); + const errorResultIndex = results.findIndex((result) => isQueryExecutionErrorResult(result)); const activeResultIndex = results.findIndex((result) => result.columns.length > 0); const resultIndex = errorResultIndex >= 0 ? errorResultIndex : (preservedResultIndex(results, current.activeResultIndex, options?.preserveActiveResultIndex) ?? (activeResultIndex >= 0 ? activeResultIndex : 0)); current.results = results;