From 2124cccef7ee7bbaffd5d6babbd42b32d8596cd1 Mon Sep 17 00:00:00 2001 From: zipg Date: Mon, 24 Aug 2026 12:50:45 +0800 Subject: [PATCH] fix(oracle): support editing query results without primary key --- .../src/lib/metadata/tableMetadataCache.ts | 8 ++ .../queryStore.hiddenPrimaryKey.spec.ts | 82 +++++++++++++++++++ apps/desktop/src/stores/queryStore.ts | 30 ++++++- 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/lib/metadata/tableMetadataCache.ts b/apps/desktop/src/lib/metadata/tableMetadataCache.ts index e57dd006b4..888abb0616 100644 --- a/apps/desktop/src/lib/metadata/tableMetadataCache.ts +++ b/apps/desktop/src/lib/metadata/tableMetadataCache.ts @@ -172,6 +172,14 @@ export function getCachedTableMetadata(request: Pick, tableType: string): boolean { + const scope = tableMetadataScope(request); + const hit = tableMetadataCache.get(scope); + if (!hit) return false; + tableMetadataCache.set(scope, { ...hit.value, tableType }, { cachedAt: hit.cachedAt }); + return true; +} + export function getCachedTableColumns(request: Pick): TableColumnsLoadResult | undefined { const hit = tableColumnsCache.get(tableMetadataScope(request)); if (!hit) return undefined; diff --git a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts index 684587fdd0..f646d5a9fc 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts @@ -10,6 +10,7 @@ const analyzeEditableQueryEditability = vi.fn(); const getColumns = vi.fn(); const listIndexes = vi.fn(); const listObjects = vi.fn(); +const listTables = vi.fn(); const getConnectionConfig = vi.fn(); const lookupLocalCompletionTables = vi.fn(); const buildSortedQuerySql = vi.fn(); @@ -49,6 +50,7 @@ vi.mock("@/lib/backend/api", () => ({ getColumns, listIndexes, listObjects, + listTables, prepareQueryPaginationExecutionPlan, saveOpenTabsState: vi.fn().mockResolvedValue(undefined), })); @@ -96,6 +98,7 @@ describe("queryStore hidden primary key editing", () => { ]); listIndexes.mockResolvedValue([]); listObjects.mockResolvedValue([]); + listTables.mockResolvedValue([]); lookupLocalCompletionTables.mockReturnValue([]); analyzeEditableQueryEditability.mockImplementation(async (sql: string) => queryAnalysis(sql)); buildSortedQuerySql.mockImplementation(async (options) => ({ ok: true, sql: `${options.originalSql} ORDER BY ${options.column} ${options.direction.toUpperCase()}` })); @@ -566,6 +569,85 @@ describe("queryStore hidden primary key editing", () => { expect(store.tabs.find((item) => item.id === tabId)?.result?.hidden_column_indexes).toEqual([2]); }); + it("resolves the Oracle current schema before adding ROWID to an unqualified keyless table query", async () => { + getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); + getColumns.mockResolvedValue([ + { name: "ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: false, extra: null }, + { name: "NAME", data_type: "VARCHAR2(100)", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + listTables.mockResolvedValue([{ name: "CUSTOMERS", table_type: "TABLE" }]); + analyzeEditableQueryEditability.mockImplementation(async (sql: string) => { + const hidden = sql.includes("__DBX_PK_0"); + return { + editable: true, + analysis: { + schema: undefined, + tableName: "CUSTOMERS", + selectStar: !hidden, + columns: hidden + ? [ + { star: true, sourceKey: "CUSTOMERS:0", resultName: "*", expression: "*" }, + { resultName: "__DBX_PK_0", expression: "ROWIDTOCHAR(ROWID)" }, + ] + : [], + }, + }; + }); + executeMulti.mockResolvedValue([ + { + columns: ["ID", "NAME", "__DBX_PK_0"], + rows: [[1, "Alice", "AAAPr9AAEAAAACXAAA"]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + store.setAutoCommit(tabId, true); + + await store.executeTabSql(tabId, "SELECT * FROM CUSTOMERS"); + + expect(listTables).toHaveBeenCalledWith("oracle-1", "ORCL", "", "CUSTOMERS"); + expect(executeMulti).toHaveBeenCalledWith("oracle-1", "ORCL", 'SELECT CUSTOMERS.*, ROWIDTOCHAR(ROWID) AS "__DBX_PK_0" FROM CUSTOMERS', undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 })); + expect(store.tabs.find((item) => item.id === tabId)?.result?.hidden_column_indexes).toEqual([2]); + await vi.waitFor(() => expect(store.tabs.find((item) => item.id === tabId)?.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"])); + + await store.executeTabSql(tabId, "SELECT * FROM CUSTOMERS"); + expect(listTables).toHaveBeenCalledOnce(); + }); + + it("keeps an unqualified Oracle view read-only after resolving it in the current schema", async () => { + getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); + getColumns.mockResolvedValue([ + { name: "ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: false, extra: null }, + { name: "NAME", data_type: "VARCHAR2(100)", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + listTables.mockResolvedValue([{ name: "CUSTOMERS", table_type: "VIEW" }]); + analyzeEditableQueryEditability.mockResolvedValue({ + editable: true, + analysis: { + schema: undefined, + tableName: "CUSTOMERS", + selectStar: true, + columns: [], + }, + }); + executeMulti.mockResolvedValue([{ columns: ["ID", "NAME"], rows: [[1, "Alice"]], affected_rows: 0, execution_time_ms: 1 }]); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + store.setAutoCommit(tabId, true); + + await store.executeTabSql(tabId, "SELECT * FROM CUSTOMERS"); + + expect(listTables).toHaveBeenCalledWith("oracle-1", "ORCL", "", "CUSTOMERS"); + expect(executeMulti).toHaveBeenCalledWith("oracle-1", "ORCL", "SELECT * FROM CUSTOMERS", undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 })); + expect(store.tabs.find((item) => item.id === tabId)?.result?.hidden_column_indexes).toBeUndefined(); + }); + it("does not check Oracle ROWID eligibility when query metadata returns no columns", async () => { getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); getColumns.mockResolvedValue([]); diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index da4ac492d1..d6444f3415 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -42,7 +42,7 @@ import { tableMetaForDataTab } from "@/lib/table/tableDataTabMeta"; import { isDataTabMetadataLifecycleStale } from "@/lib/sidebar/dataTabOpenPolicy"; import { dataTabExecutionDatabase } from "@/lib/table/dataTabExecutionDatabase"; import { tableOpenPageLimit } from "@/lib/table/tableOpenPageLimit"; -import { getCachedTableMetadata, loadTableColumns, loadTableIndexes, loadTableMetadata, tableMetadataToDataTabMeta, type TableMetadataRequest } from "@/lib/metadata/tableMetadataCache"; +import { getCachedTableMetadata, loadTableColumns, loadTableIndexes, loadTableMetadata, tableMetadataToDataTabMeta, updateCachedTableMetadataType, type TableMetadataRequest } from "@/lib/metadata/tableMetadataCache"; import { MetadataTaskLimiter } from "@/lib/metadata/metadataTaskLimiter"; import { buildTableSelectSql, quoteTableDataIdentifier } from "@/lib/table/tableSelectSql"; import { connectionObjectTreeNodeSchema, connectionQueryExecutionSchema, connectionUsesDatabaseObjectTreeMode, effectiveDatabaseTypeForConnection, gaussdbCountQueryDopHint, metadataSchemaForConnection } from "@/lib/database/jdbcDialect"; @@ -3848,6 +3848,32 @@ export const useQueryStore = defineStore("query", () => { return matches.length === 1 && matches[0]?.type === "table"; } + async function resolveOracleRowIdSafety(tab: QueryTab, loaded: LoadedEditableSource): Promise { + if (oracleRowIdIsSafeForQuery(tab, loaded)) return true; + if (loaded.tableMeta.tableType?.trim()) return false; + + const connection = useConnectionStore().getConfig(tab.connectionId!); + const schema = loaded.tableMeta.schema?.trim() || tab.schema?.trim() || connection?.default_schema?.trim() || ""; + const tables = await api.listTables(tab.connectionId!, loaded.tableMeta.database ?? tab.database, schema, loaded.tableMeta.tableName); + const exactMatches = tables.filter((table) => table.name === loaded.tableMeta.tableName); + if (exactMatches.length !== 1) return false; + + loaded.tableMeta.tableType = exactMatches[0]!.table_type; + updateCachedTableMetadataType( + { + connectionId: tab.connectionId!, + database: loaded.tableMeta.database ?? tab.database, + schema: loaded.tableMeta.schema, + tableName: loaded.tableMeta.tableName, + databaseType: "oracle", + driverProfile: connection?.driver_profile || connection?.db_type, + catalog: loaded.tableMeta.catalog, + }, + loaded.tableMeta.tableType, + ); + return oracleRowIdIsSafeForQuery(tab, loaded); + } + function primaryKeyIndex(indexes: IndexInfo[]): IndexInfo | undefined { return indexes.find((index) => !index.filter && index.columns.length > 0 && index.is_primary); } @@ -3921,7 +3947,7 @@ export const useQueryStore = defineStore("query", () => { // Oracle base tables without a natural identifier use the same ROWID // identity as table-data tabs. Confirm the object is a base table because // selecting ROWID from a view can fail with ORA-01445. - if (syntheticOracleRowId && !oracleRowIdIsSafeForQuery(tab, loaded)) return unchanged; + if (syntheticOracleRowId && !(await resolveOracleRowIdSafety(tab, loaded))) return unchanged; const declaredPrimaryKeys = databaseType === "oracle" && !syntheticOracleRowId ? primaryKeys : columnPrimaryKeys; return buildHiddenPrimaryKeyPreparation(tab, sql, databaseType, loaded, primaryKeys, declaredPrimaryKeys, traceId, elapsed); } catch (error) {