diff --git a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts index 3c2a38021d..7c943e691a 100644 --- a/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts +++ b/apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts @@ -249,7 +249,7 @@ describe("editable query hidden primary keys", () => { expect(sourceColumnsForResult(aliased.analysis, ["ID", "NUM", "ROWNUM"], undefined, "oracle")).toEqual(["id", "amount", undefined]); }); - it("maps a qualified Oracle ROWID projection to the synthetic row key", () => { + it.each(["oracle", "oceanbase-oracle"] as const)("maps a qualified %s ROWID projection to the synthetic row key", (databaseType) => { const analysis: EditableQueryInfo = { schema: "APP", tableName: "USERS", @@ -261,9 +261,9 @@ describe("editable query hidden primary keys", () => { ], }; - expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "t:0", "oracle")).toBe(true); - expect(sourceColumnsForResult(analysis, ["ROWID", "NAME", "LABEL"], "t:0", "oracle", ["__DBX_ROWID"])).toEqual(["__DBX_ROWID", "NAME", undefined]); - expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "o:1", "oracle")).toBe(false); + expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "t:0", databaseType)).toBe(true); + expect(sourceColumnsForResult(analysis, ["ROWID", "NAME", "LABEL"], "t:0", databaseType, ["__DBX_ROWID"])).toEqual(["__DBX_ROWID", "NAME", undefined]); + expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "o:1", databaseType)).toBe(false); }); it("does not assign an unqualified multi-source ROWID to a target table", () => { diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index ac92cbf1b0..a9182662eb 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -897,7 +897,7 @@ export function allPrimaryKeysPresent(primaryKeys: string[], resultColumns: stri analysis.columns.flatMap((column) => { if (!column.sourceName) return []; if (sourceKey && column.sourceKey !== sourceKey) return []; - if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) return [DBX_ROWID_COLUMN, column.sourceName]; + if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) return [DBX_ROWID_COLUMN, column.sourceName]; return [column.sourceName]; }), ); @@ -981,7 +981,7 @@ export function sourceColumnsForResult(analysis: EditableQueryInfo, resultColumn return matchedColumns.map((column) => { if (!column) return undefined; if (sourceKey && column.sourceKey !== sourceKey) return undefined; - if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) { + if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) { return primaryKeys?.length === 1 && primaryKeys[0] === DBX_ROWID_COLUMN ? DBX_ROWID_COLUMN : undefined; } return column.sourceName; diff --git a/apps/desktop/src/stores/__tests__/queryStore.oceanbaseRowId.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.oceanbaseRowId.spec.ts new file mode 100644 index 0000000000..a97c513ada --- /dev/null +++ b/apps/desktop/src/stores/__tests__/queryStore.oceanbaseRowId.spec.ts @@ -0,0 +1,177 @@ +import { createPinia, setActivePinia } from "pinia"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + executeMulti: vi.fn(), + executeQuery: vi.fn(), + getColumns: vi.fn(), + listIndexes: vi.fn(), + listObjects: vi.fn(), + listTables: vi.fn(), + getConfig: vi.fn(), + lookupLocalCompletionTables: vi.fn(), + analyze: vi.fn(), + preparePlan: vi.fn(), +})); +vi.stubGlobal("localStorage", { getItem: () => null, setItem: () => {}, removeItem: () => {} }); +vi.mock("@/lib/backend/api", () => ({ + analyzeEditableQueryEditability: mocks.analyze, + executeMulti: mocks.executeMulti, + executeQuery: mocks.executeQuery, + getColumns: mocks.getColumns, + listIndexes: mocks.listIndexes, + listObjects: mocks.listObjects, + listTables: mocks.listTables, + prepareQueryPaginationExecutionPlan: mocks.preparePlan, + closeClientConnectionSession: vi.fn().mockResolvedValue(undefined), + closeQuerySession: vi.fn().mockResolvedValue(undefined), + saveOpenTabsState: vi.fn(), + cancelQuery: vi.fn(), +})); +vi.mock("@/stores/connectionStore", () => ({ + useConnectionStore: () => ({ + ensureConnected: vi.fn().mockResolvedValue(undefined), + getConfig: mocks.getConfig, + lookupLocalCompletionTables: mocks.lookupLocalCompletionTables, + recordConnectionLostError: vi.fn(), + metadataGenerationFor: () => 0, + }), +})); +vi.mock("@/stores/settingsStore", () => ({ + useSettingsStore: () => ({ + editorSettings: { pageSize: 100, autoCalculateTotalRows: false }, + }), +})); + +describe("OceanBase Oracle query ROWID preparation", () => { + beforeEach(async () => { + vi.clearAllMocks(); + setActivePinia(createPinia()); + const { clearTableMetadataCache } = await import("@/lib/metadata/tableMetadataCache"); + clearTableMetadataCache(); + mocks.getConfig.mockReturnValue({ id: "ob-1", name: "OceanBase", db_type: "oceanbase-oracle", database: "app", query_timeout_secs: 30 }); + mocks.getColumns.mockResolvedValue([{ name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, column_default: null, is_primary_key: false, extra: null }]); + mocks.listIndexes.mockResolvedValue([]); + mocks.listTables.mockResolvedValue([]); + mocks.listObjects.mockResolvedValue([]); + mocks.lookupLocalCompletionTables.mockReturnValue([{ name: "T_SIPF_DEBUG_LOG", type: "table", schema: "SIPF" }]); + mocks.analyze.mockResolvedValue({ editable: false, reason: "complex-source" }); + mocks.preparePlan.mockImplementation(async (options) => ({ sqlToExecute: options.sql, useAgentResultSession: false })); + mocks.executeMulti.mockResolvedValue([{ columns: ["TASKNAME"], rows: [["sample"]], affected_rows: 0, execution_time_ms: 1 }]); + }); + + async function executedQuery(sql: string) { + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const id = store.createTab("ob-1", "app", "Query"); + store.setAutoCommit(id, true); + await store.executeTabSql(id, sql); + expect(mocks.executeMulti).toHaveBeenCalled(); + return { store, id, sql: mocks.executeMulti.mock.calls[0]![2] as string }; + } + + async function emittedSql(sql: string) { + return (await executedQuery(sql)).sql; + } + + it("appends a ROWID expression to a single-column projection of a keyless table", async () => { + const sql = await emittedSql("select a.taskname from sipf.t_sipf_debug_log a"); + expect(sql).toContain('ROWIDTOCHAR(ROWID) AS "__DBX_PK_0"'); + expect(sql).not.toContain('"__DBX_ROWID"'); + }); + + it.each(["select * from sipf.t_sipf_debug_log a", "select a.rowid, a.taskname from sipf.t_sipf_debug_log a", "select q.taskname from (select a.taskname from sipf.t_sipf_debug_log a) q"])("preserves queries not needing an extra ROWID: %s", async (sql) => { + expect(await emittedSql(sql)).toBe(sql); + }); + + it.each(["view", "unknown"])("does not inject ROWID for a %s source", async (type) => { + mocks.lookupLocalCompletionTables.mockReturnValue(type === "unknown" ? [] : [{ name: "T_SIPF_DEBUG_LOG", type, schema: "SIPF" }]); + const sql = "select a.taskname from sipf.t_sipf_debug_log a"; + expect(await emittedSql(sql)).toBe(sql); + expect(mocks.listTables).not.toHaveBeenCalled(); + }); + + it("prefers a physical primary key", async () => { + mocks.getColumns.mockResolvedValue([ + { name: "ID", data_type: "NUMBER", is_nullable: false, is_primary_key: true }, + { name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false }, + ]); + const sql = await emittedSql("select a.taskname from sipf.t_sipf_debug_log a"); + expect(sql).toContain('"ID" AS "__DBX_PK_0"'); + expect(sql).not.toContain("ROWID"); + }); + + it("hides the projected ROWID while retaining two row identities for editing", async () => { + const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis"); + mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql)); + mocks.executeMulti.mockResolvedValue([ + { + columns: ["TASKNAME", "__DBX_PK_0"], + rows: [ + ["first", "AAAPr9AAEAAAACXAAA"], + ["second", "AAAPr9AAEAAAACXAAB"], + ], + affected_rows: 0, + execution_time_ms: 1, + }, + ]); + + const { store, id } = await executedQuery("select a.taskname from sipf.t_sipf_debug_log a"); + const tab = store.tabs.find((item) => item.id === id)!; + expect(tab.result?.hidden_column_indexes).toEqual([1]); + expect(tab.result?.rows).toEqual([ + ["first", "AAAPr9AAEAAAACXAAA"], + ["second", "AAAPr9AAEAAAACXAAB"], + ]); + await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["TASKNAME", "__DBX_ROWID"])); + expect(tab.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"]); + expect(tab.queryEditabilityReason).toBeUndefined(); + }); + + it("does not confuse a quoted physical rowid column with the synthetic identity", async () => { + mocks.getColumns.mockResolvedValue([ + { name: "rowid", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false }, + { name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false }, + ]); + const sql = await emittedSql('select a."rowid", a.taskname from sipf.t_sipf_debug_log a'); + expect(sql).toBe('select a."rowid", a.taskname, ROWIDTOCHAR(ROWID) AS "__DBX_PK_0" from sipf.t_sipf_debug_log a'); + }); + + it("uses an explicitly selected ROWID as the row identity without hiding it", async () => { + const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis"); + mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql)); + mocks.executeMulti.mockResolvedValue([{ columns: ["RID", "TASKNAME"], rows: [["AAAPr9AAEAAAACXAAA", "first"]], affected_rows: 0, execution_time_ms: 1 }]); + const originalSql = "select a.rowid as rid, a.taskname from sipf.t_sipf_debug_log a"; + const { store, id, sql } = await executedQuery(originalSql); + expect(sql).toBe(originalSql); + const tab = store.tabs.find((item) => item.id === id)!; + expect(tab.result?.hidden_column_indexes ?? []).toEqual([]); + await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["__DBX_ROWID", "TASKNAME"])); + expect(tab.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"]); + }); + + it("retains the physical primary key when ROWID is also selected", async () => { + const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis"); + mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql)); + mocks.getColumns.mockResolvedValue([ + { name: "ID", data_type: "NUMBER", is_nullable: false, is_primary_key: true }, + { name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false }, + ]); + mocks.executeMulti.mockResolvedValue([{ columns: ["RID", "TASKNAME", "__DBX_PK_0"], rows: [["AAAPr9AAEAAAACXAAA", "first", 1]], affected_rows: 0, execution_time_ms: 1 }]); + const { store, id, sql } = await executedQuery("select a.rowid as rid, a.taskname from sipf.t_sipf_debug_log a"); + expect(sql).toContain('"ID" AS "__DBX_PK_0"'); + expect(sql).not.toContain("ROWIDTOCHAR"); + const tab = store.tabs.find((item) => item.id === id)!; + await vi.waitFor(() => expect(tab.querySourceColumns).toEqual([undefined, "TASKNAME", "ID"])); + expect(tab.tableMeta?.primaryKeys).toEqual(["ID"]); + }); + + it.each([ + "select distinct a.taskname from sipf.t_sipf_debug_log a", + "select a.taskname, count(*) from sipf.t_sipf_debug_log a group by a.taskname", + "select a.taskname, b.taskname from sipf.t_sipf_debug_log a join sipf.t_sipf_debug_log b on a.taskname = b.taskname", + "select a.taskname from sipf.t_sipf_debug_log a union select b.taskname from sipf.t_sipf_debug_log b", + ])("does not inject ROWID into an unsafe query: %s", async (sql) => { + expect(await emittedSql(sql)).toBe(sql); + }); +}); diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 43e3fd40ba..75c865a17b 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -902,7 +902,7 @@ function bindColumnsForSource( if (!column.sourceName) return column; if (column.sourceKey) { if (column.sourceKey !== source.key) return column; - if (dbType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return column; + if ((dbType === "oracle" || dbType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return column; const canonicalName = resolveSourceColumnName(dbType, column.sourceName, column.sourceNameQuoted, tableColumns); return { ...column, sourceName: canonicalName }; } @@ -6319,7 +6319,7 @@ export const useQueryStore = defineStore("query", () => { const selectedColumns = new Set( analysis.columns.flatMap((column) => { if (!column.sourceName || column.sourceKey !== sourceKey) return []; - if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return [DBX_ROWID_COLUMN, column.sourceName]; + if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return [DBX_ROWID_COLUMN, column.sourceName]; return [column.sourceName]; }), ); @@ -6345,9 +6345,9 @@ export const useQueryStore = defineStore("query", () => { async function resolveOracleRowIdSafety(tab: QueryTab, loaded: LoadedEditableSource, databaseType: DatabaseType): Promise { if (oracleRowIdIsSafeForQuery(tab, loaded)) return true; if (loaded.tableMeta.tableType?.trim()) return false; - // Never enumerate an Oracle schema on the query execution path: large + // Never enumerate an Oracle/OB schema on the query execution path: large // schemas can make this optional editability check take minutes (#8462). - if (databaseType === "oracle") return false; + if (databaseType === "oracle" || databaseType === "oceanbase-oracle") return false; const connection = useConnectionStore().getConfig(tab.connectionId!); const schema = loaded.tableMeta.schema?.trim() || tab.schema?.trim() || connection?.default_schema?.trim() || ""; @@ -6382,7 +6382,9 @@ export const useQueryStore = defineStore("query", () => { const unchanged = { sql, metadataSql: sql, hiddenPrimaryKeys: [], largeValuePreview }; const missingPrimaryKeys = declaredPrimaryKeys.length === 0 - ? primaryKeys.filter((primaryKey) => !(databaseType === "oracle" && primaryKey === DBX_ROWID_COLUMN && metadataAnalysis.columns.some((column) => column.sourceKey === loaded.source.key && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID"))) + ? primaryKeys.filter( + (primaryKey) => !((databaseType === "oracle" || databaseType === "oceanbase-oracle") && primaryKey === DBX_ROWID_COLUMN && metadataAnalysis.columns.some((column) => column.sourceKey === loaded.source.key && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID")), + ) : missingPrimaryKeysForSource(databaseType, primaryKeys, metadataAnalysis, loaded.source.key); if (missingPrimaryKeys.length === 0) return unchanged; const primaryKeySet = new Set(primaryKeys); @@ -6394,7 +6396,7 @@ export const useQueryStore = defineStore("query", () => { databaseType, primaryKeys: missingPrimaryKeys, existingResultNames: metadataAnalysis.selectStar ? loaded.tableMeta.columns.map((column) => column.name) : metadataAnalysis.columns.map((column) => column.resultName), - sourceExpressions: missingPrimaryKeys.includes(DBX_ROWID_COLUMN) && (databaseType === "oracle" || databaseType === "xugu") ? { [DBX_ROWID_COLUMN]: databaseType === "oracle" ? "ROWIDTOCHAR(ROWID)" : "ROWID" } : undefined, + sourceExpressions: missingPrimaryKeys.includes(DBX_ROWID_COLUMN) && (databaseType === "oracle" || databaseType === "oceanbase-oracle" || databaseType === "xugu") ? { [DBX_ROWID_COLUMN]: databaseType === "xugu" ? "ROWID" : "ROWIDTOCHAR(ROWID)" } : undefined, }); if (!rewritten) return unchanged; queryExecutionLog("info", "hidden-primary-keys", { @@ -6474,9 +6476,9 @@ export const useQueryStore = defineStore("query", () => { if (loaded.tableMeta.columns.length === 0) return unchanged; if (loaded.tableMeta.tableType?.toUpperCase().includes("VIEW")) return unchanged; const primaryKeys = loaded.tableMeta.primaryKeys; - const syntheticRowId = (databaseType === "oracle" || databaseType === "xugu") && usesSyntheticRowIdKey(databaseType, primaryKeys, loaded.tableMeta.tableType); + const syntheticRowId = (databaseType === "oracle" || databaseType === "oceanbase-oracle" || databaseType === "xugu") && usesSyntheticRowIdKey(databaseType, primaryKeys, loaded.tableMeta.tableType); // Base tables without a natural identifier use the same ROWID identity - // as table-data tabs (Oracle and Xugu). Confirm the object is a base + // as table-data tabs (Oracle, OceanBase Oracle and Xugu). Confirm the object is a base // table because selecting ROWID from a view can fail with ORA-01445. if (syntheticRowId && !(await resolveOracleRowIdSafety(tab, loaded, databaseType))) return unchanged; const declaredPrimaryKeys = syntheticRowId ? [] : primaryKeys;