diff --git a/apps/desktop/src/lib/__tests__/table/tableEditing.spec.ts b/apps/desktop/src/lib/__tests__/table/tableEditing.spec.ts index 99e3e0780e..a85020c495 100644 --- a/apps/desktop/src/lib/__tests__/table/tableEditing.spec.ts +++ b/apps/desktop/src/lib/__tests__/table/tableEditing.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + DBX_NEO4J_ELEMENT_ID_COLUMN, DBX_ROWID_COLUMN, DBX_TDENGINE_TBNAME_COLUMN, canDeleteExistingTdengineRows, @@ -75,6 +76,25 @@ describe("tableEditing", () => { expect(editableRowIdentifierColumns("postgres", [column("id", true), column("email")], [index(["email"])])).toEqual(["id"]); }); + it.each(["oracle", "oceanbase-oracle"] as const)("prefers physical %s indexes over the ROWID fallback", (databaseType) => { + const columns = [column("OFFER_RELA_ID"), column("ORI_OFFER_ID")]; + const primaryIndex = { ...index(["OFFER_RELA_ID"], false), is_primary: true }; + + expect(editableRowIdentifierColumns(databaseType, columns, [index(["ORI_OFFER_ID"]), primaryIndex], "TABLE")).toEqual(["OFFER_RELA_ID"]); + expect(editableRowIdentifierColumns(databaseType, columns, [index(["ORI_OFFER_ID"])], "TABLE")).toEqual(["ORI_OFFER_ID"]); + expect(editableRowIdentifierColumns(databaseType, columns, [index(["ORI_OFFER_ID"], false)], "TABLE")).toEqual([DBX_ROWID_COLUMN]); + expect(editableRowIdentifierColumns(databaseType, columns, [index(["ORI_OFFER_ID"], true, "ORI_OFFER_ID IS NOT NULL")], "TABLE")).toEqual([DBX_ROWID_COLUMN]); + expect(editableRowIdentifierColumns(databaseType, columns, [], "TABLE")).toEqual([DBX_ROWID_COLUMN]); + }); + + it("keeps synthetic row identifiers scoped to their existing fallbacks", () => { + const columns = [column("ID"), column("NAME")]; + + expect(editableRowIdentifierColumns("oracle", columns, [], "VIEW")).toEqual([]); + expect(editableRowIdentifierColumns("oracle", columns, [], "MATERIALIZED_VIEW")).toEqual([]); + expect(editableRowIdentifierColumns("neo4j", columns, [index(["ID"])], "TABLE")).toEqual([DBX_NEO4J_ELEMENT_ID_COLUMN]); + }); + it("allows ClickHouse table editing when row identifiers are available", () => { expect(isTableDataEditable("clickhouse", ["id"], "BASE TABLE")).toBe(true); expect(canEditExistingTableRows("clickhouse", undefined, ["id"])).toBe(true); diff --git a/apps/desktop/src/lib/table/tableEditing.ts b/apps/desktop/src/lib/table/tableEditing.ts index 375f75c8e2..5f02268330 100644 --- a/apps/desktop/src/lib/table/tableEditing.ts +++ b/apps/desktop/src/lib/table/tableEditing.ts @@ -26,9 +26,10 @@ export function editablePrimaryKeys(databaseType: DatabaseType | undefined, colu export function editableRowIdentifierColumns(databaseType: DatabaseType | undefined, columns: ColumnInfo[], indexes?: IndexInfo[], tableType?: string): string[] { const primaryKeys = editablePrimaryKeys(databaseType, columns, tableType); - if (primaryKeys.length > 0) return primaryKeys; + const oracleRowIdFallback = getDatabaseCapability(databaseType).syntheticKey === "oracle-rowid" && primaryKeys.length === 1 && primaryKeys[0]?.toUpperCase() === DBX_ROWID_COLUMN; + if (primaryKeys.length > 0 && !oracleRowIdFallback) return primaryKeys; const uniqueIndex = indexes?.filter((index) => !index.filter && index.columns.length > 0 && (index.is_primary || index.is_unique)).sort((left, right) => Number(right.is_primary) - Number(left.is_primary) || left.columns.length - right.columns.length)[0]; - return uniqueIndex?.columns ?? []; + return uniqueIndex?.columns ?? primaryKeys; } export function isTableDataEditable(databaseType: DatabaseType | undefined, primaryKeys: string[], tableType?: string): boolean { diff --git a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts index 8bc6f28825..684587fdd0 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts @@ -440,6 +440,84 @@ describe("queryStore hidden primary key editing", () => { expect(tab.queryEditabilityReason).toBeUndefined(); }); + it.each([ + { + connection: { id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }, + label: "native Oracle", + }, + { + connection: { id: "oracle-jdbc-1", name: "Oracle JDBC", db_type: "jdbc", connection_string: "jdbc:oracle:thin:@//localhost:1521/ORCL", database: "ORCL", query_timeout_secs: 30 }, + label: "Oracle-inferred JDBC", + }, + ])("uses a physical primary index for $label star projections when column flags omit it", async ({ connection }) => { + getConnectionConfig.mockReturnValue(connection); + getColumns.mockResolvedValue([ + { name: "OFFER_RELA_ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: false, extra: null }, + { name: "ORI_OFFER_ID", data_type: "NUMBER", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + listIndexes.mockResolvedValue([{ name: "PK_OFFER_RELA", columns: ["OFFER_RELA_ID"], is_unique: true, is_primary: true }]); + lookupLocalCompletionTables.mockReturnValue([{ name: "OFFER_RELA", type: "table", schema: "APP" }]); + + const projections = [ + { + sql: "SELECT T.* FROM APP.OFFER_RELA t", + analysisColumns: [], + resultColumns: ["OFFER_RELA_ID", "ORI_OFFER_ID"], + sourceColumns: undefined, + selectStar: true, + }, + { + sql: "SELECT T.*, T.ROWID FROM APP.OFFER_RELA t", + analysisColumns: [ + { star: true, sourceQualifier: "T", sourceKey: "t:0", resultName: "*", expression: "T.*" }, + { sourceName: "ROWID", sourceNameQuoted: false, sourceQualifier: "T", sourceKey: "t:0", resultName: "ROWID", expression: "T.ROWID" }, + ], + resultColumns: ["OFFER_RELA_ID", "ORI_OFFER_ID", "ROWID"], + sourceColumns: ["OFFER_RELA_ID", "ORI_OFFER_ID", undefined], + selectStar: false, + }, + ]; + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + + for (const projection of projections) { + analyzeEditableQueryEditability.mockResolvedValue({ + editable: true, + analysis: { + schema: "APP", + tableName: "OFFER_RELA", + tableAlias: "t", + selectStar: projection.selectStar, + columns: projection.analysisColumns, + }, + }); + executeMulti.mockResolvedValue([ + { + columns: projection.resultColumns, + rows: [], + affected_rows: 0, + execution_time_ms: 1, + }, + ]); + + const tabId = store.createTab(connection.id, "ORCL", "Query"); + store.setAutoCommit(tabId, true); + await store.executeTabSql(tabId, projection.sql); + + expect(executeMulti).toHaveBeenLastCalledWith(connection.id, "ORCL", projection.sql, undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 })); + const tab = store.tabs.find((item) => item.id === tabId)!; + if (projection.sourceColumns) { + await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(projection.sourceColumns)); + } else { + expect(tab.querySourceColumns).toBeUndefined(); + } + await vi.waitFor(() => expect(tab.tableMeta?.primaryKeys).toEqual(["OFFER_RELA_ID"])); + expect(tab.queryAnalysis).toBeDefined(); + expect(tab.queryEditabilityReason).toBeUndefined(); + } + }); + it("uses the configured current schema to keep an unqualified Oracle base-table query editable", async () => { getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", default_schema: "APP", query_timeout_secs: 30 }); getColumns.mockResolvedValue([ diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index be8bec25c4..f883a93a66 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -3915,12 +3915,14 @@ export const useQueryStore = defineStore("query", () => { loaded ??= await loadEditableQuerySource(tab, analysis, sources[0]!, conn, databaseType, executionDatabase, traceId, elapsed); if (loaded.tableMeta.columns.length === 0) return unchanged; if (loaded.tableMeta.tableType?.toUpperCase().includes("VIEW")) return unchanged; - const declaredPrimaryKeys = loaded.tableMeta.columns.filter((column) => column.is_primary_key).map((column) => column.name); - // Oracle base tables without declared keys 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 (databaseType === "oracle" && declaredPrimaryKeys.length === 0 && !oracleRowIdIsSafeForQuery(tab, loaded)) return unchanged; - const primaryKeys = editablePrimaryKeys(databaseType, loaded.tableMeta.columns, loaded.tableMeta.tableType); + const columnPrimaryKeys = loaded.tableMeta.columns.filter((column) => column.is_primary_key).map((column) => column.name); + const primaryKeys = databaseType === "oracle" ? loaded.tableMeta.primaryKeys : editablePrimaryKeys(databaseType, loaded.tableMeta.columns, loaded.tableMeta.tableType); + const syntheticOracleRowId = databaseType === "oracle" && usesSyntheticRowIdKey(databaseType, primaryKeys, loaded.tableMeta.tableType); + // 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; + const declaredPrimaryKeys = databaseType === "oracle" && !syntheticOracleRowId ? primaryKeys : columnPrimaryKeys; return buildHiddenPrimaryKeyPreparation(tab, sql, databaseType, loaded, primaryKeys, declaredPrimaryKeys, traceId, elapsed); } catch (error) { // Metadata enrichment is optional. Query execution must retain its prior