From 98df9756140a9709be340b71622d946a22ececb2 Mon Sep 17 00:00:00 2001 From: Melody Ruch <396921921@qq.com> Date: Sat, 15 Aug 2026 14:42:06 +0800 Subject: [PATCH] fix(sqlserver): focus data results in multi-target execution --- .../useSqlExecutionMultiDbFocus.spec.ts | 288 ++++++++++++++++++ .../src/composables/useSqlExecution.ts | 24 +- apps/desktop/src/stores/queryStore.ts | 2 +- .../sqlserverResultFocus6189.test.ts | 151 +++++++++ 4 files changed, 458 insertions(+), 7 deletions(-) create mode 100644 apps/desktop/src/composables/__tests__/useSqlExecutionMultiDbFocus.spec.ts create mode 100644 packages/app-tests/sqlserverResultFocus6189.test.ts diff --git a/apps/desktop/src/composables/__tests__/useSqlExecutionMultiDbFocus.spec.ts b/apps/desktop/src/composables/__tests__/useSqlExecutionMultiDbFocus.spec.ts new file mode 100644 index 0000000000..3107a9d8c0 --- /dev/null +++ b/apps/desktop/src/composables/__tests__/useSqlExecutionMultiDbFocus.spec.ts @@ -0,0 +1,288 @@ +/** + * Issue #6189 — SQL Server "focus jumps to the message result". + * + * Background: + * - `crates/dbx-core/src/db/sqlserver.rs:523-542` synthesizes a pseudo result with a + * single "Message" column and `server_message: true` for any batch segment that + * produced only server messages (PRINT / "DBCC execution completed" / ...). + * - `stores/queryStore.ts:4727` then picks the first result that HAS COLUMNS as the + * active one. The synthesized message grid has a column, so a message result can win. + * - `composables/useSqlExecution.ts` (`focusSqlServerDataResult`, shared by both + * `doExecute` and `executeTargetSql`) corrects that: when the selected result is a + * `server_message` one it re-focuses the first real data result. Before this fix, + * `executeTargetSql` (the multi-database execute path) had no such correction. + */ +import { computed, ref } from "vue"; +import { createPinia, setActivePinia } from "pinia"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useSqlExecution } from "../useSqlExecution"; +import { useHistoryStore } from "@/stores/historyStore"; +import { useQueryStore } from "@/stores/queryStore"; +import type { ConnectionConfig, QueryTab } from "@/types/database"; + +vi.mock("vue-i18n", () => ({ + createI18n: () => ({ global: { locale: { value: "en" }, setLocaleMessage: vi.fn() } }), + useI18n: () => ({ t: (key: string) => key }), +})); + +vi.mock("@/lib/backend/api", () => ({ + saveEditorSettings: vi.fn(), + saveHistory: vi.fn(), +})); + +function installLocalStorage() { + const data = new Map(); + vi.stubGlobal("localStorage", { + getItem: vi.fn((key: string) => data.get(key) ?? null), + setItem: vi.fn((key: string, value: string) => data.set(key, value)), + removeItem: vi.fn((key: string) => data.delete(key)), + }); +} + +function sqlServerConnection(): ConnectionConfig { + return { + id: "conn-1", + name: "SQLServer", + db_type: "sqlserver", + host: "localhost", + port: 1433, + username: "sa", + password: "", + }; +} + +function queryTab(database = "dbx_sqlserver_demo"): QueryTab { + return { + id: "tab-1", + connectionId: "conn-1", + database, + schema: undefined, + title: "SQL", + sql: "", + mode: "query", + isDirty: false, + isExecuting: false, + isCancelling: false, + isExplaining: false, + }; +} + +/** + * What the SQL Server driver really returns for `PRINT N'x'; SELECT 1 AS value;`: + * the PRINT becomes a synthesized single-column "Message" grid flagged + * `server_message: true`, followed by the real SELECT result. + */ +function sqlServerMessageFirstResults() { + const messageResult = { + columns: ["Message"], + column_types: ["nvarchar"], + rows: [["x"]], + affected_rows: 0, + execution_time_ms: 1, + server_message: true, + } as const; + const dataResult = { + columns: ["value"], + column_types: ["int"], + rows: [[1]], + affected_rows: 0, + execution_time_ms: 1, + }; + return { messageResult, dataResult }; +} + +describe("SQL Server result focus: doExecute vs executeTargetSql", () => { + beforeEach(() => { + installLocalStorage(); + setActivePinia(createPinia()); + }); + + // ---- control: the single-connection editor path (fixed by be3336c1e) ---- + it("CONTROL doExecute re-focuses the real data result when a message result was selected", async () => { + const sql = "PRINT N'x'; SELECT 1 AS value;"; + const tab = { ...queryTab(), sql }; + const activeTab = ref(tab); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + const { messageResult, dataResult } = sqlServerMessageFirstResults(); + + const setActiveResultIndex = vi.spyOn(queryStore, "setActiveResultIndex").mockImplementation((_id, index) => { + if (!tab.results) return; + tab.activeResultIndex = index; + tab.result = tab.results[index]; + }); + vi.spyOn(queryStore, "executeCurrentSql").mockImplementation(async () => { + tab.results = [messageResult, dataResult]; + // queryStore.ts:4727 — first result WITH COLUMNS wins, and the message grid has one. + tab.activeResultIndex = 0; + tab.result = messageResult; + return true; + }); + vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => activeTab.value), + activeConnection: computed(() => sqlServerConnection()), + executableSql: computed(() => sql), + activeOutputView, + }); + + await execution.tryExecute(); + + expect(setActiveResultIndex).toHaveBeenCalledWith("tab-1", 1); + expect(tab.activeResultIndex).toBe(1); + expect(tab.result?.server_message).toBeUndefined(); + expect(tab.result?.rows).toEqual([[1]]); + }); + + it("executeTargetSql focuses the real data result, like doExecute (fix for #6189)", async () => { + const sql = "PRINT N'x'; SELECT 1 AS value;"; + const tab = { ...queryTab(), sql }; + const connection = sqlServerConnection(); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + const { messageResult, dataResult } = sqlServerMessageFirstResults(); + + vi.spyOn(queryStore, "setActiveResultIndex").mockImplementation((_id, index) => { + if (!tab.results) return; + tab.activeResultIndex = index; + tab.result = tab.results[index]; + }); + vi.spyOn(queryStore, "executeTabSql").mockImplementation(async () => { + tab.results = [messageResult, dataResult]; + tab.activeResultIndex = 0; + tab.result = messageResult; + return true; + }); + vi.spyOn(queryStore, "getExecutionTab").mockReturnValue(tab); + vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => tab as QueryTab | undefined), + activeConnection: computed(() => connection), + executableSql: computed(() => sql), + activeOutputView, + }); + + await execution.executeTargetSql({ tab, connection, sql }); + + expect(tab.activeResultIndex).toBe(1); + expect(tab.result?.server_message).toBeUndefined(); + expect(tab.result?.rows).toEqual([[1]]); + }); + + // ---- the "trailing message" shape actually described by #6189 / #5566 ---- + it("executeTargetSql keeps the first data result when the message result is last", async () => { + const sql = "SELECT 1 AS a; SELECT 2 AS b; PRINT N'DBCC execution completed.';"; + const tab = { ...queryTab(), sql }; + const connection = sqlServerConnection(); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + + const first = { columns: ["a"], column_types: ["int"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }; + const second = { columns: ["b"], column_types: ["int"], rows: [[2]], affected_rows: 0, execution_time_ms: 1 }; + const trailingMessage = { + columns: ["Message"], + column_types: ["nvarchar"], + rows: [["DBCC execution completed."]], + affected_rows: 0, + execution_time_ms: 1, + server_message: true, + } as const; + + vi.spyOn(queryStore, "executeTabSql").mockImplementation(async () => { + tab.results = [first, second, trailingMessage]; + tab.activeResultIndex = 0; + tab.result = first; + return true; + }); + vi.spyOn(queryStore, "getExecutionTab").mockReturnValue(tab); + vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => tab as QueryTab | undefined), + activeConnection: computed(() => connection), + executableSql: computed(() => sql), + activeOutputView, + }); + + await execution.executeTargetSql({ tab, connection, sql }); + + expect(tab.activeResultIndex).toBe(0); + expect(tab.result?.rows).toEqual([[1]]); + }); +}); + +describe("preservedResultIndex staleness theory", () => { + beforeEach(() => { + installLocalStorage(); + setActivePinia(createPinia()); + }); + + it("doExecute never asks the store to preserve the previous active result index", async () => { + const sql = "SELECT 1 AS a; SELECT 2 AS b; PRINT N'trailing';"; + const tab = { ...queryTab(), sql, results: undefined, activeResultIndex: 2 } as QueryTab; + const activeTab = ref(tab); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + + const executeCurrentSql = vi.spyOn(queryStore, "executeCurrentSql").mockImplementation(async () => { + tab.results = [ + { columns: ["a"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }, + { columns: ["Message"], rows: [["trailing"]], affected_rows: 0, execution_time_ms: 1, server_message: true }, + ]; + tab.activeResultIndex = 0; + tab.result = tab.results[0]; + return true; + }); + vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => activeTab.value), + activeConnection: computed(() => sqlServerConnection()), + executableSql: computed(() => sql), + activeOutputView, + }); + + await execution.tryExecute(); + + const options = executeCurrentSql.mock.calls[0]?.[1] ?? {}; + // queryStore.ts:238-241 only reuses the previous index when this flag is true, + // so without it a new execution can never inherit a stale "last card" index. + expect(options).not.toHaveProperty("preserveActiveResultIndex"); + expect(tab.activeResultIndex).toBe(0); + }); + + it("executeTargetSql never asks the store to preserve the previous active result index", async () => { + const sql = "SELECT 1 AS a; PRINT N'trailing';"; + const tab = { ...queryTab(), sql, activeResultIndex: 2 } as QueryTab; + const connection = sqlServerConnection(); + const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result"); + const queryStore = useQueryStore(); + + const executeTabSql = vi.spyOn(queryStore, "executeTabSql").mockImplementation(async () => { + tab.results = [ + { columns: ["a"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }, + { columns: ["Message"], rows: [["trailing"]], affected_rows: 0, execution_time_ms: 1, server_message: true }, + ]; + tab.activeResultIndex = 0; + tab.result = tab.results[0]; + return true; + }); + vi.spyOn(queryStore, "getExecutionTab").mockReturnValue(tab); + vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined); + + const execution = useSqlExecution({ + activeTab: computed(() => tab as QueryTab | undefined), + activeConnection: computed(() => connection), + executableSql: computed(() => sql), + activeOutputView, + }); + + await execution.executeTargetSql({ tab, connection, sql }); + + const options = executeTabSql.mock.calls[0]?.[2] ?? {}; + expect(options).not.toHaveProperty("preserveActiveResultIndex"); + expect(tab.activeResultIndex).toBe(0); + }); +}); diff --git a/apps/desktop/src/composables/useSqlExecution.ts b/apps/desktop/src/composables/useSqlExecution.ts index 8ccc92b275..c2db1f16cb 100644 --- a/apps/desktop/src/composables/useSqlExecution.ts +++ b/apps/desktop/src/composables/useSqlExecution.ts @@ -252,6 +252,22 @@ export function useSqlExecution(deps: { return false; } + // SQL Server batches that end in PRINT/DBCC-style messages with no rows of their own get + // synthesized into a "Message" pseudo-result (server_message: true). The store's generic + // "first result with columns" pick can land on that pseudo-result instead of real data, so + // whenever it does, redirect focus to the first real data result (falling back to the + // message itself only if there is no data result to show). Shared by every SQL execution + // entry point so none of them can regress independently (see #6189). + function focusSqlServerDataResult(executionTabId: string, executionDatabaseType: DatabaseType | undefined, tab: Pick) { + if (executionDatabaseType !== "sqlserver") return; + const sqlServerMessageResultIndex = tab.results?.findIndex((result) => result.server_message === true); + if (sqlServerMessageResultIndex === undefined || sqlServerMessageResultIndex < 0) return; + const activeSqlServerResult = tab.results && tab.activeResultIndex !== undefined ? tab.results[tab.activeResultIndex] : tab.result; + if (activeSqlServerResult?.server_message !== true) return; + const sqlServerDataResultIndex = tab.results?.findIndex((result) => result.server_message !== true && !isQueryExecutionErrorResult(result) && result.columns.length > 0); + queryStore.setActiveResultIndex(executionTabId, sqlServerDataResultIndex !== undefined && sqlServerDataResultIndex >= 0 ? sqlServerDataResultIndex : sqlServerMessageResultIndex); + } + async function doExecute(sql?: string, sourceOffset?: number, options: SqlExecutionOptions = {}) { if (sql === undefined) ({ sql, sourceOffset } = await resolvedExecutableSql()); const tab = deps.activeTab.value; @@ -274,13 +290,8 @@ export function useSqlExecution(deps: { }); if (producedResult === false) return; const sqlServerMessageResultIndex = executionDatabaseType === "sqlserver" ? tab.results?.findIndex((result) => result.server_message === true) : undefined; - const activeSqlServerResult = tab.results && tab.activeResultIndex !== undefined ? tab.results[tab.activeResultIndex] : tab.result; if (sqlServerMessageResultIndex !== undefined && sqlServerMessageResultIndex >= 0) { - if (activeSqlServerResult?.server_message === true) { - const sqlServerDataResultIndex = tab.results?.findIndex((result) => result.server_message !== true && !isQueryExecutionErrorResult(result) && result.columns.length > 0); - if (sqlServerDataResultIndex !== undefined && sqlServerDataResultIndex >= 0) queryStore.setActiveResultIndex(tab.id, sqlServerDataResultIndex); - else queryStore.setActiveResultIndex(tab.id, sqlServerMessageResultIndex); - } + focusSqlServerDataResult(tab.id, executionDatabaseType, tab); deps.activeOutputView.value = "result"; } else if (executionDatabaseType === "sqlserver" && tab.result?.server_message === true) { deps.activeOutputView.value = "result"; @@ -425,6 +436,7 @@ export function useSqlExecution(deps: { if (cancelRequested() || tabCancelRequested(cancelRequestCount)) { return finish({ status: "cancelled" }); } + focusSqlServerDataResult(executionTabId, connection.db_type, latest); const failure = firstQueryExecutionError(latest); const errorMessage = failure ? String(failure.rows?.[0]?.[0] ?? t("common.failed")) : undefined; const success = !failure; diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index a4f6f84149..60f14a9780 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -5368,7 +5368,7 @@ export const useQueryStore = defineStore("query", () => { } function setActiveResultIndex(id: string, index: number) { - const tab = tabs.value.find((t) => t.id === id); + const tab = findExecutionTab(id); if (!tab?.results || index < 0 || index >= tab.results.length) return; tab.activeResultIndex = index; tab.result = tab.results[index]; diff --git a/packages/app-tests/sqlserverResultFocus6189.test.ts b/packages/app-tests/sqlserverResultFocus6189.test.ts new file mode 100644 index 0000000000..7d288d2968 --- /dev/null +++ b/packages/app-tests/sqlserverResultFocus6189.test.ts @@ -0,0 +1,151 @@ +/** + * Issue #6189 investigation — "focus jumps to the last result set" on SQL Server. + * + * Exercises the REAL queryStore.executeTabSql over two consecutive executions to test + * the theory that `preservedResultIndex` (queryStore.ts:238-241) can leak a previously + * focused index onto a later, differently-shaped result set. + */ +import { strict as assert } from "node:assert"; +import { afterEach, test } from "vitest"; +import { createPinia, disposePinia, getActivePinia, setActivePinia } from "pinia"; +import { useConnectionStore } from "../../apps/desktop/src/stores/connectionStore.ts"; +import { useQueryStore } from "../../apps/desktop/src/stores/queryStore.ts"; +import type { ConnectionConfig } from "../../apps/desktop/src/types/database.ts"; + +afterEach(() => { + const pinia = getActivePinia(); + if (pinia) disposePinia(pinia); + setActivePinia(undefined); +}); + +function installMemoryStorage() { + const values = new Map(); + const original = Object.getOwnPropertyDescriptor(globalThis, "localStorage"); + Object.defineProperty(globalThis, "localStorage", { + configurable: true, + value: { + getItem: (key: string) => values.get(key) ?? null, + setItem: (key: string, value: string) => values.set(key, value), + removeItem: (key: string) => values.delete(key), + clear: () => values.clear(), + }, + }); + return () => { + if (original) Object.defineProperty(globalThis, "localStorage", original); + else Reflect.deleteProperty(globalThis, "localStorage"); + }; +} + +function sqlServerConn(id: string): ConnectionConfig { + return { + id, + name: id, + db_type: "sqlserver", + host: "localhost", + port: 1433, + username: "sa", + password: "", + }; +} + +const THREE_RESULTS = "SELECT 1 AS a; SELECT 2 AS b; SELECT 3 AS c;"; +const TRAILING_MESSAGE = "SELECT 9 AS z; PRINT N'DBCC execution completed.';"; + +/** Mirrors the shapes crates/dbx-core/src/db/sqlserver.rs:523-542 actually produces. */ +function resultsFor(sql: string) { + if (sql.includes("SELECT 3 AS c")) { + return [ + { columns: ["a"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }, + { columns: ["b"], rows: [[2]], affected_rows: 0, execution_time_ms: 1 }, + { columns: ["c"], rows: [[3]], affected_rows: 0, execution_time_ms: 1 }, + ]; + } + return [ + { columns: ["z"], rows: [[9]], affected_rows: 0, execution_time_ms: 1 }, + { + columns: ["Message"], + column_types: ["nvarchar"], + rows: [["DBCC execution completed."]], + affected_rows: 0, + execution_time_ms: 1, + server_message: true, + }, + ]; +} + +function stubFetch(record: string[]): typeof fetch { + return async (input, init) => { + const url = String(input); + const json = (value: unknown) => new Response(JSON.stringify(value), { status: 200, headers: { "Content-Type": "application/json" } }); + if (url === "/api/connection/check-health") return json(null); + if (url === "/api/query/prepare-pagination-plan") { + const body = JSON.parse(String(init?.body ?? "{}")); + return json({ sqlToExecute: body.options.sql, useAgentResultSession: false }); + } + if (url === "/api/query/execute-multi") { + const body = JSON.parse(String(init?.body ?? "{}")); + return json(resultsFor(String(body.sql ?? ""))); + } + if (url === "/api/query/analyze-editability") return json({ editable: false, reason: "complex-source" }); + record.push(url); + return json(null); + }; +} + +test("#6189: a later SQL Server execution does not inherit the previously focused result index", async () => { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + const unexpected: string[] = []; + globalThis.fetch = stubFetch(unexpected); + + connectionStore.addEphemeralConnection({ ...sqlServerConn("conn-1"), database: "dbx" }); + const tabId = store.createTab("conn-1", "dbx", "Query"); + + try { + // 1) three data results; the user manually focuses the LAST one. + await store.executeTabSql(tabId, THREE_RESULTS); + let tab = store.tabs.find((item) => item.id === tabId); + assert.equal(tab?.results?.length, 3, "first execution should produce three results"); + assert.equal(tab?.activeResultIndex, 0, "first execution focuses the first result with columns"); + + store.setActiveResultIndex(tabId, 2); + tab = store.tabs.find((item) => item.id === tabId); + assert.equal(tab?.activeResultIndex, 2, "user selected the third card"); + + // 2) a DIFFERENT batch: one data result plus a trailing server-message pseudo-result. + await store.executeTabSql(tabId, TRAILING_MESSAGE); + tab = store.tabs.find((item) => item.id === tabId); + assert.equal(tab?.results?.length, 2, "second execution should produce two results"); + assert.equal(tab?.activeResultIndex, 0, "the stale index must not leak onto the new result set"); + assert.equal(tab?.result?.server_message, undefined, "the trailing message must not be focused"); + assert.deepEqual(tab?.result?.rows, [[9]]); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } +}); + +test("#6189: a SQL Server batch whose message result is last still focuses the first data result", async () => { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + globalThis.fetch = stubFetch([]); + + connectionStore.addEphemeralConnection({ ...sqlServerConn("conn-1"), database: "dbx" }); + const tabId = store.createTab("conn-1", "dbx", "Query"); + + try { + await store.executeTabSql(tabId, TRAILING_MESSAGE); + const tab = store.tabs.find((item) => item.id === tabId); + assert.equal(tab?.activeResultIndex, 0); + assert.deepEqual(tab?.result?.rows, [[9]]); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } +});