From 47e02b8da7a3915d5269b6420756f3ad93d06065 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 21 Aug 2026 19:44:13 +0800 Subject: [PATCH] fix(oracle): defer XMLTYPE in manual transactions --- agents/drivers/oracle-go/main.go | 6 +- agents/drivers/oracle-go/main_test.go | 18 +++ apps/desktop/src/lib/backend/http.ts | 2 +- apps/desktop/src/lib/backend/tauri.ts | 3 +- .../queryStore.hiddenPrimaryKey.spec.ts | 119 +++++++++++++++++- apps/desktop/src/stores/queryStore.ts | 11 +- crates/dbx-core/src/query.rs | 61 ++++++++- src-tauri/src/commands/query.rs | 6 +- 8 files changed, 209 insertions(+), 17 deletions(-) diff --git a/agents/drivers/oracle-go/main.go b/agents/drivers/oracle-go/main.go index c15a24ea9e..ff8e3e39dc 100644 --- a/agents/drivers/oracle-go/main.go +++ b/agents/drivers/oracle-go/main.go @@ -4379,7 +4379,7 @@ func rewriteOracleSelectItems(items []string, columns []oracleColumnMeta, tableR for _, column := range columns { columnRef := oracleColumnRef(tableRef.AliasText, column.Name) outputAlias := quoteIdentifier(column.Name) - if isOracleXMLType(column.DataType) { + if isOracleXMLType(column.DataType) && !deferLOBs { rewritten = append(rewritten, oracleXMLSerializeExpression(columnRef, outputAlias)) } else if deferLOBs { if expressions, ok := oracleDeferredLOBExpressions(columnRef, outputAlias, sourceIndex, column.DataType); ok { @@ -4403,7 +4403,7 @@ func rewriteOracleSelectItems(items []string, columns []oracleColumnMeta, tableR outputAlias = quoteIdentifier(meta.Name) } columnRef := oracleColumnRef(qualifier, meta.Name) - if isOracleXMLType(meta.DataType) { + if isOracleXMLType(meta.DataType) && !deferLOBs { rewritten = append(rewritten, oracleXMLSerializeExpression(columnRef, outputAlias)) changed = true sourceIndex++ @@ -4446,6 +4446,8 @@ func oracleDeferredLOBKind(dataType string) (kind, placeholder string, ok bool) return "L", "", true case "BFILE": return "F", "", true + case "XMLTYPE", "SYS.XMLTYPE": + return "C", "", true default: return "", "", false } diff --git a/agents/drivers/oracle-go/main_test.go b/agents/drivers/oracle-go/main_test.go index 21a8f7d457..d464eb8083 100644 --- a/agents/drivers/oracle-go/main_test.go +++ b/agents/drivers/oracle-go/main_test.go @@ -2288,6 +2288,24 @@ func TestRewriteOracleLOBSelectStarAsDeferredValues(t *testing.T) { } } +func TestRewriteOracleXMLTypeAsDeferredValue(t *testing.T) { + sqlText, err := rewriteOracleSelectSQL( + `SELECT t.ID, t.XML_CONTENT FROM TEST_LOBS t WHERE t.ID = 1`, + fakeOracleColumnLoader([]oracleColumnMeta{ + {Name: "ID", DataType: "NUMBER"}, + {Name: "XML_CONTENT", DataType: "SYS.XMLTYPE"}, + }), + true, + ) + if err != nil { + t.Fatal(err) + } + want := `SELECT t.ID, CASE WHEN t."XML_CONTENT" IS NULL THEN NULL ELSE '' END AS "XML_CONTENT", CASE WHEN t."XML_CONTENT" IS NULL THEN NULL ELSE 'D:1' END AS "__DBX_LARGE_VALUE_BYTES_C_1" FROM TEST_LOBS t WHERE t.ID = 1` + if sqlText != want { + t.Fatalf("rewriteOracleSelectSQL() = %s, want %s", sqlText, want) + } +} + func TestRewriteOracleLOBExplicitColumnUsesVisibleResultIndex(t *testing.T) { sqlText, err := rewriteOracleSelectSQL( `SELECT t.ID, t.PAYLOAD AS body, LENGTH(t.PAYLOAD) AS payload_length FROM TEST_LOBS t`, diff --git a/apps/desktop/src/lib/backend/http.ts b/apps/desktop/src/lib/backend/http.ts index 8e24ece2f4..6e17bcd1fb 100644 --- a/apps/desktop/src/lib/backend/http.ts +++ b/apps/desktop/src/lib/backend/http.ts @@ -1216,7 +1216,7 @@ export async function beginManualTransaction(_connectionId: string, _database: s throw new Error("Manual transaction management is only available in the desktop app."); } -export async function executeInManualTransaction(_txnSessionId: string, _sql: string, _database: string, _schema?: string, _maxRows?: number): Promise { +export async function executeInManualTransaction(_txnSessionId: string, _sql: string, _database: string, _schema?: string, _maxRows?: number, _tableDataPreview?: boolean): Promise { throw new Error("Manual transaction management is only available in the desktop app."); } diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index c82864f04c..8422b2bca6 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -1390,13 +1390,14 @@ export async function beginManualTransaction(connectionId: string, database: str return invoke("begin_manual_transaction", { connectionId, database, schema, catalog }); } -export async function executeInManualTransaction(txnSessionId: string, sql: string, database: string, schema?: string, maxRows?: number): Promise { +export async function executeInManualTransaction(txnSessionId: string, sql: string, database: string, schema?: string, maxRows?: number, tableDataPreview?: boolean): Promise { return invoke("execute_in_manual_transaction", { txnSessionId, sql, database, schema, maxRows, + tableDataPreview, }); } diff --git a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts index 66653bf6b8..8bc6f28825 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts @@ -3,6 +3,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const executeMulti = vi.fn(); const executeQuery = vi.fn(); +const beginManualTransaction = vi.fn(); +const executeInManualTransaction = vi.fn(); +const cancelQuery = vi.fn(); const analyzeEditableQueryEditability = vi.fn(); const getColumns = vi.fn(); const listIndexes = vi.fn(); @@ -38,6 +41,9 @@ vi.mock("@/lib/backend/api", () => ({ buildSortedQuerySql, closeClientConnectionSession: vi.fn().mockResolvedValue(undefined), closeQuerySession: vi.fn().mockResolvedValue(undefined), + beginManualTransaction, + cancelQuery, + executeInManualTransaction, executeMulti, executeQuery, getColumns, @@ -110,6 +116,7 @@ describe("queryStore hidden primary key editing", () => { affected_rows: 0, execution_time_ms: 1, }); + cancelQuery.mockResolvedValue(false); executeMulti.mockResolvedValue([ { columns: ["name", "__DBX_PK_0"], @@ -118,6 +125,15 @@ describe("queryStore hidden primary key editing", () => { execution_time_ms: 1, }, ]); + beginManualTransaction.mockResolvedValue("txn-1"); + executeInManualTransaction.mockResolvedValue([ + { + columns: ["name", "__DBX_PK_0"], + rows: [["Alice", 7]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]); }); afterEach(() => { @@ -580,6 +596,7 @@ describe("queryStore hidden primary key editing", () => { const execution = store.executeTabSql(tabId, "SELECT t.* FROM APP.WIDE_TABLE t"); await vi.waitFor(() => expect(executeMulti).toHaveBeenCalled()); expect(executeMulti).toHaveBeenCalledWith("oracle-1", "ORCL", "SELECT t.* FROM APP.WIDE_TABLE t", undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 })); + expect(executeMulti.mock.calls[0]?.[5]).not.toHaveProperty("tableDataPreview"); columnsGate.resolve([ { name: "ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: true, extra: null }, @@ -588,6 +605,104 @@ describe("queryStore hidden primary key editing", () => { await execution; }); + it("waits for first-run Oracle XMLTYPE metadata before manual transaction execution", async () => { + const columnsGate = deferred>>(); + getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); + getColumns.mockReturnValue(columnsGate.promise); + listIndexes.mockResolvedValue([{ name: "PK_WIDE_TABLE", columns: ["ID"], is_unique: true, is_primary: true }]); + lookupLocalCompletionTables.mockReturnValue([{ name: "WIDE_TABLE", type: "table", schema: "APP" }]); + analyzeEditableQueryEditability.mockImplementation(async () => ({ + editable: true, + analysis: { + schema: "APP", + tableName: "WIDE_TABLE", + tableAlias: "t", + selectStar: true, + columns: [], + }, + })); + executeInManualTransaction.mockResolvedValue([ + { + columns: ["ID", "PAYLOAD"], + rows: [[1, ""]], + affected_rows: 0, + execution_time_ms: 1, + large_value_cells: [{ row_index: 0, column_index: 1, original_bytes: 81920 }], + }, + ]); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + const execution = store.executeTabSql(tabId, "SELECT t.* FROM APP.WIDE_TABLE t"); + + await vi.waitFor(() => expect(listIndexes).toHaveBeenCalled()); + expect(executeInManualTransaction).not.toHaveBeenCalled(); + + columnsGate.resolve([ + { name: "ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: true, extra: null }, + { name: "PAYLOAD", data_type: "SYS.XMLTYPE", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + await execution; + + expect(beginManualTransaction).toHaveBeenCalledWith("oracle-1", "ORCL", undefined, undefined); + expect(executeInManualTransaction).toHaveBeenCalledWith("txn-1", "SELECT t.* FROM APP.WIDE_TABLE t", "ORCL", undefined, expect.any(Number), true); + expect(store.tabs.find((tab) => tab.id === tabId)?.result?.large_value_cells).toEqual([{ row_index: 0, column_index: 1, original_bytes: 81920 }]); + }); + + it("does not start an Oracle manual transaction after cancellation during metadata loading", async () => { + const columnsGate = deferred>>(); + getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); + getColumns.mockReturnValue(columnsGate.promise); + listIndexes.mockResolvedValue([{ name: "PK_WIDE_TABLE", columns: ["ID"], is_unique: true, is_primary: true }]); + lookupLocalCompletionTables.mockReturnValue([{ name: "WIDE_TABLE", type: "table", schema: "APP" }]); + analyzeEditableQueryEditability.mockImplementation(async () => ({ + editable: true, + analysis: { + schema: "APP", + tableName: "WIDE_TABLE", + tableAlias: "t", + selectStar: true, + columns: [], + }, + })); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + const execution = store.executeTabSql(tabId, "SELECT t.* FROM APP.WIDE_TABLE t"); + + await vi.waitFor(() => expect(listIndexes).toHaveBeenCalled()); + await expect(store.cancelTabExecution(tabId)).resolves.toBe(false); + + columnsGate.resolve([ + { name: "ID", data_type: "NUMBER", is_nullable: false, column_default: null, is_primary_key: true, extra: null }, + { name: "PAYLOAD", data_type: "SYS.XMLTYPE", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + ]); + await expect(execution).resolves.toBe(false); + + expect(cancelQuery).toHaveBeenCalledWith(expect.any(String)); + expect(beginManualTransaction).not.toHaveBeenCalled(); + expect(executeInManualTransaction).not.toHaveBeenCalled(); + expect(store.tabs.find((tab) => tab.id === tabId)).toMatchObject({ + isExecuting: false, + isCancelling: false, + executionId: undefined, + }); + expect(store.tabs.find((tab) => tab.id === tabId)?.txnSessionId).toBeUndefined(); + }); + + it("keeps manual non-Oracle queries out of table-data preview mode", async () => { + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("mysql-1", "app", "Query"); + store.setAutoCommit(tabId, false); + + await store.executeTabSql(tabId, "SELECT name FROM users"); + + expect(executeInManualTransaction).toHaveBeenCalledWith("txn-1", "SELECT name, `id` AS `__DBX_PK_0` FROM users", "app", undefined, expect.any(Number), false); + }); + it("keeps a keyless Oracle query editable when its WHERE clause reads another table", async () => { getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 }); getColumns.mockResolvedValue([ @@ -686,11 +801,11 @@ describe("queryStore hidden primary key editing", () => { expect(tab.result?.hidden_column_indexes).toBeUndefined(); }); - it("enables deferred Oracle LOBs only when a base-table query has a stable key", async () => { + it.each(["CLOB", "XMLTYPE", "SYS.XMLTYPE"])("enables deferred Oracle %s values only when a base-table query has a stable key", async (dataType) => { 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: true, extra: null }, - { name: "PAYLOAD", data_type: "CLOB", is_nullable: true, column_default: null, is_primary_key: false, extra: null }, + { name: "PAYLOAD", data_type: dataType, is_nullable: true, column_default: null, is_primary_key: false, extra: null }, ]); lookupLocalCompletionTables.mockReturnValue([{ name: "DOCUMENTS", type: "table", schema: "APP" }]); analyzeEditableQueryEditability.mockImplementation(async (sql: string) => ({ diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index a246dbd653..235e72d966 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -83,7 +83,7 @@ import type { DriverProfileWorkspaceScope } from "@/lib/database/driverProfileEx import type { MultiDbExecutionTarget, MultiDbResultRunExecution } from "@/types/sqlExecution"; const ORACLE_LIKE_METADATA_TYPES = new Set(["oracle", "dameng", "oceanbase-oracle"]); -const ORACLE_DEFERRED_LOB_TYPES = new Set(["CLOB", "NCLOB", "BLOB", "BFILE"]); +const ORACLE_DEFERRED_LOB_TYPES = new Set(["CLOB", "NCLOB", "BLOB", "BFILE", "XMLTYPE", "SYS.XMLTYPE"]); // Bounded concurrency for grouped-query display column loads, scoped per // connection so different connections never block each other. Matches the @@ -3789,7 +3789,7 @@ export const useQueryStore = defineStore("query", () => { }); void fullMetadataPromise.catch((error) => queryExecutionLog("warn", "metadata:table-prefetch:failed", { traceId, error, elapsed: elapsed() })); const indexes = await loadTableIndexes(target.request); - if (primaryKeyIndex(indexes) && projectsAllColumnsForSource(target.analysis, target.source.key)) { + if (primaryKeyIndex(indexes) && projectsAllColumnsForSource(target.analysis, target.source.key) && tab.autoCommit !== false) { return unchanged; } loaded = loadedEditableSourceFromMetadata(target, (await fullMetadataPromise).metadata); @@ -4929,6 +4929,11 @@ export const useQueryStore = defineStore("query", () => { const frontendTimeoutSecs = frontendQueryTimeoutSecsForSql(sqlToExecute, effectiveDbType, queryTimeoutSecs); const sourceLabelDatabase = targetDatabase || conn?.database; const executionClientSessionId = options?.pagination?.clientSessionId ?? (tab.mode === "query" || tab.mode === "data" ? tabClientSessionId(tab) : undefined); + const currentBeforeDispatch = findExecutionTab(id); + if (currentBeforeDispatch?.executionId !== executionId || currentBeforeDispatch.isCancelling) { + queryExecutionLog("info", "dispatch:skipped-cancelled", { traceId, elapsed: elapsed() }); + return false; + } let executionPromise: Promise; if (tab.autoCommit === false) { @@ -4939,7 +4944,7 @@ export const useQueryStore = defineStore("query", () => { } queryExecutionLog("info", "execute-in-txn:invoke", { traceId, txnSessionId: tab.txnSessionId, elapsed: elapsed() }); executionDispatched = true; - executionPromise = api.executeInManualTransaction(tab.txnSessionId, sqlToExecute, executionDatabase, executionSchema, pageLimit ?? agentProtocolQueryResultMaxRows(queryResultMaxRows)); + executionPromise = api.executeInManualTransaction(tab.txnSessionId, sqlToExecute, executionDatabase, executionSchema, pageLimit ?? agentProtocolQueryResultMaxRows(queryResultMaxRows), useOracleLobPreview); } else { queryExecutionLog("info", "execute-multi:start", { traceId, elapsed: elapsed() }); // Query and data tabs use a tab-scoped pool so repeated executions keep diff --git a/crates/dbx-core/src/query.rs b/crates/dbx-core/src/query.rs index 48843da5e0..7d0d3a7054 100644 --- a/crates/dbx-core/src/query.rs +++ b/crates/dbx-core/src/query.rs @@ -4502,10 +4502,24 @@ pub async fn execute_in_manual_transaction( state: &AppState, txn_session_id: &str, sql: &str, - _database: &str, - _schema: Option<&str>, + database: &str, + schema: Option<&str>, max_rows: Option, ) -> Result, String> { + execute_in_manual_transaction_with_options(state, txn_session_id, sql, database, schema, max_rows, false) + .await + .map(|results| results.into_iter().map(ExecuteMultiResult::into_query_result).collect()) +} + +pub async fn execute_in_manual_transaction_with_options( + state: &AppState, + txn_session_id: &str, + sql: &str, + database: &str, + schema: Option<&str>, + max_rows: Option, + table_data_preview: bool, +) -> Result, String> { const TXN_IDLE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); // Resolve statements and validate before taking the per-session connection @@ -4525,7 +4539,10 @@ pub async fn execute_in_manual_transaction( |db_type| crate::sql::split_sql_statements_for_database(sql, db_type), ); if statements.is_empty() { - return Ok(vec![empty_query_result(0)]); + return Ok(vec![ExecuteMultiResult::success_with_optional_server_large_values( + empty_query_result(0), + table_data_preview, + )]); } // Read-only check while the session is still in the map. If this fails the @@ -4577,11 +4594,25 @@ pub async fn execute_in_manual_transaction( } TxnConnection::Mysql(conn) => execute_manual_txn_mysql_statement(conn, statement, row_limit).await, TxnConnection::Agent { client, .. } => { - execute_manual_txn_agent_statement(client, db_type, statement, _database, _schema, row_limit).await + execute_manual_txn_agent_statement( + client, + db_type, + statement, + database, + schema, + row_limit, + table_data_preview, + ) + .await } }; match result { - Ok(query_result) => results.push(query_result), + Ok(query_result) => { + results.push(ExecuteMultiResult::success_with_optional_server_large_values( + query_result, + table_data_preview, + )); + } Err(e) => { // Statement failure ends the transaction. If another cleanup path // already removed the session, it owns the final rollback. @@ -4792,6 +4823,10 @@ async fn release_manual_txn_agent_pool(state: &AppState, connection_id: &str, co } } +fn manual_txn_agent_query_options(row_limit: usize, table_data_preview: bool) -> QueryExecutionOptions { + QueryExecutionOptions { max_rows: Some(row_limit.max(1)), table_data_preview, ..QueryExecutionOptions::default() } +} + async fn execute_manual_txn_agent_statement( client: &Arc, db_type: Option, @@ -4799,10 +4834,11 @@ async fn execute_manual_txn_agent_statement( database: &str, schema: Option<&str>, row_limit: usize, + table_data_preview: bool, ) -> Result { let sql = sql_for_execution_context(db_type, statement, schema); let execution_schema = schema_for_execution_context(db_type, schema); - let options = QueryExecutionOptions { max_rows: Some(row_limit.max(1)), ..QueryExecutionOptions::default() }; + let options = manual_txn_agent_query_options(row_limit, table_data_preview); let params = agent_execute_query_params(&sql, Some(database).filter(|value| !value.is_empty()), execution_schema, options); let mut locked = client.lock().await; @@ -7314,6 +7350,19 @@ for line in sys.stdin: assert_eq!(params["deferLobs"], true); } + #[test] + fn manual_transaction_agent_query_options_preserve_preview_mode() { + let params = agent_execute_query_params( + "SELECT * FROM documents", + Some("ORCL"), + Some("APP"), + manual_txn_agent_query_options(250, true), + ); + + assert_eq!(params["maxRows"], 250); + assert_eq!(params["deferLobs"], true); + } + #[test] fn iris_execution_context_omits_schema() { assert_eq!(schema_for_execution_context(Some(DatabaseType::Iris), Some("SQLUser")), None); diff --git a/src-tauri/src/commands/query.rs b/src-tauri/src/commands/query.rs index 2da1378ecc..02479d467b 100644 --- a/src-tauri/src/commands/query.rs +++ b/src-tauri/src/commands/query.rs @@ -455,14 +455,16 @@ pub async fn execute_in_manual_transaction( database: String, schema: Option, max_rows: Option, -) -> Result, String> { - dbx_core::query::execute_in_manual_transaction( + table_data_preview: Option, +) -> Result, String> { + dbx_core::query::execute_in_manual_transaction_with_options( &state, &txn_session_id, &sql, &database, schema.as_deref(), max_rows, + table_data_preview.unwrap_or(false), ) .await }