From 920ab62cf11ab83671273e360c438d5400009b1b Mon Sep 17 00:00:00 2001 From: zipg Date: Thu, 27 Aug 2026 16:28:14 +0800 Subject: [PATCH] feat(grid): support deleting rows from JOIN query results --- .../src/components/layout/ContentArea.vue | 2 +- apps/desktop/src/lib/sql/sqlAnalysis.ts | 1 + apps/desktop/src/stores/queryStore.ts | 5 +- apps/desktop/src/types/database.ts | 1 + crates/dbx-core/src/data_grid_sql.rs | 35 ++++ packages/app-tests/queryResultToolbar.test.ts | 4 +- packages/app-tests/queryStore.test.ts | 163 ++++++++++++++++++ 7 files changed, 207 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/components/layout/ContentArea.vue b/apps/desktop/src/components/layout/ContentArea.vue index 2628f88cbc..2954e300b9 100644 --- a/apps/desktop/src/components/layout/ContentArea.vue +++ b/apps/desktop/src/components/layout/ContentArea.vue @@ -1763,7 +1763,7 @@ defineExpose({ :mongo-update-target="mongoQueryResultSaveHandler && activeTab.result.mongo_copy_documents?.length === activeTab.result.rows.length ? activeTab.mongoEditTarget : undefined" :query-editability-reason="activeTab.queryEditabilityReason" :allow-insert-rows="activeTab.queryAnalysis?.allowInsert ?? activeTab.queryAnalysis?.allowInsertDelete !== false" - :allow-delete-rows="activeTab.queryAnalysis?.allowInsertDelete !== false" + :allow-delete-rows="activeTab.queryAnalysis?.allowDelete ?? activeTab.queryAnalysis?.allowInsertDelete !== false" context="results" :auto-transpose-single-row="settingsStore.editorSettings.dataGridAutoTransposeSingleRow" :database-type="activeEffectiveDatabaseType" diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index b7383691a7..7f29879624 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -23,6 +23,7 @@ export interface EditableQueryInfo { editableSourceKey?: string; multiSource?: boolean; allowInsert?: boolean; + allowDelete?: boolean; allowInsertDelete?: boolean; distinct?: boolean; groupByColumns?: EditableQueryColumn[]; diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index f48f89a483..f982bd0459 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -4229,6 +4229,8 @@ export const useQueryStore = defineStore("query", () => { const allSourceColumns = loadedSources.map((source) => ({ source: source.source, columns: source.tableMeta.columns })); // Match DBeaver's safety model: a joined result is writable only when one // source table has a complete row identifier and at least one writable column. + // A keyless source has no row identifier: allPrimaryKeysPresent is vacuously + // true for an empty key set, so joined results must exclude such sources. const candidates = loadedSources .map((loaded) => { const metadataAnalysis = expandStarProjectionColumnsForSource(bindColumnsForSource(dbType, loaded.analysis, loaded.source, loaded.tableMeta.columns, allSourceColumns), loaded.source, loaded.tableMeta.columns); @@ -4247,7 +4249,7 @@ export const useQueryStore = defineStore("query", () => { editableSourceColumnCount, }; }) - .filter((loaded) => (loaded.primaryKeysPresent || loaded.keylessAllowed) && !!loaded.sourceColumns && loaded.editableSourceColumnCount > 0); + .filter((loaded) => ((loaded.primaryKeysPresent && loaded.tableMeta.primaryKeys.length > 0) || loaded.keylessAllowed) && !!loaded.sourceColumns && loaded.editableSourceColumnCount > 0); if (loadedSources.length === 1) { const loaded = loadedSources[0]!; @@ -4330,6 +4332,7 @@ export const useQueryStore = defineStore("query", () => { const queryAnalysis = { ...target.analysis, ...(target.analysis.distinct && canInsertIntoEditableQuerySource(tab, dbType as DatabaseType, target, target.sourceColumns) ? { allowInsert: true } : {}), + allowDelete: !target.analysis.distinct, allowInsertDelete: false, multiSource: true, }; diff --git a/apps/desktop/src/types/database.ts b/apps/desktop/src/types/database.ts index e9369a30f9..8754599076 100644 --- a/apps/desktop/src/types/database.ts +++ b/apps/desktop/src/types/database.ts @@ -1253,6 +1253,7 @@ export interface QueryTab { editableSourceKey?: string; multiSource?: boolean; allowInsert?: boolean; + allowDelete?: boolean; allowInsertDelete?: boolean; distinct?: boolean; sources?: { diff --git a/crates/dbx-core/src/data_grid_sql.rs b/crates/dbx-core/src/data_grid_sql.rs index 904eca7e69..975ee9963d 100644 --- a/crates/dbx-core/src/data_grid_sql.rs +++ b/crates/dbx-core/src/data_grid_sql.rs @@ -6175,6 +6175,41 @@ mod tests { assert!(result.rollback_statements.iter().any(|statement| statement.contains(r#"WHERE "ID" = 101"#))); } + #[test] + fn mysql_join_result_delete_targets_only_the_resolved_source_primary_key() { + let result = prepare_data_grid_save(DataGridSaveStatementOptions { + database_type: Some(DatabaseType::Mysql), + identifier_quote: None, + table_meta: DataGridTableMeta { + catalog: None, + database: None, + schema: Some("lims".to_string()), + table_name: "lims_batchs_simple".to_string(), + primary_keys: vec!["id".to_string()], + columns: Some(vec![ + column("id", "bigint", false, None), + column("sno", "integer", true, None), + column("batchs_id", "bigint", false, None), + column("simple_id", "bigint", false, None), + ]), + }, + columns: vec!["id".to_string(), "sno".to_string(), "batchs_id".to_string(), "simple_id".to_string()], + source_columns: Some(vec![ + Some("id".to_string()), + Some("sno".to_string()), + Some("batchs_id".to_string()), + Some("simple_id".to_string()), + ]), + rows: vec![vec![json!(2658055), json!(4), json!(57485), json!(492045)]], + dirty_rows: vec![], + deleted_rows: vec![0], + new_rows: vec![], + }); + + assert_eq!(result.validation_error, None); + assert_eq!(result.statements, vec!["DELETE FROM `lims`.`lims_batchs_simple` WHERE `id` = 2658055;"]); + } + #[test] fn prepares_oracle_timestamp_insert_from_iso_grid_value() { let result = prepare_data_grid_save(DataGridSaveStatementOptions { diff --git a/packages/app-tests/queryResultToolbar.test.ts b/packages/app-tests/queryResultToolbar.test.ts index d0cac8c7b5..ab9a2cdd26 100644 --- a/packages/app-tests/queryResultToolbar.test.ts +++ b/packages/app-tests/queryResultToolbar.test.ts @@ -28,11 +28,11 @@ test("query result toolbar SFCs compile", () => { for (const path of [contentAreaPath, dataGridPath, viewSwitcherPath, toolbarActionsPath]) assertSfcCompiles(path); }); -test("ContentArea keeps DISTINCT inserts separate from result-row deletes", () => { +test("ContentArea keeps query-result insert and delete capabilities separate", () => { const contentArea = source(contentAreaPath); assert.match(contentArea, /:allow-insert-rows="activeTab\.queryAnalysis\?\.allowInsert \?\? activeTab\.queryAnalysis\?\.allowInsertDelete !== false"/); - assert.match(contentArea, /:allow-delete-rows="activeTab\.queryAnalysis\?\.allowInsertDelete !== false"/); + assert.match(contentArea, /:allow-delete-rows="activeTab\.queryAnalysis\?\.allowDelete \?\? activeTab\.queryAnalysis\?\.allowInsertDelete !== false"/); }); test("query result toolbar reuses the production icon contract", () => { diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index 3cfbda143b..213a2d7dba 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -3183,6 +3183,168 @@ test("keeps PostgreSQL quoted primary keys distinct from case-only result column } }); +test("allows deleting joined query rows from the single primary-key-qualified source", async () => { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + const columnRequests: string[] = []; + const sql = `SELECT bs.* FROM lims_simple s +INNER JOIN lims_batchs_simple bs ON bs.simple_id = s.id +INNER JOIN lims_batchs b ON b.id = bs.batchs_id +WHERE b.sno = 'BJ-LIMELSA-20260825-1517'`; + + connectionStore.addEphemeralConnection({ ...conn("mysql-join-delete"), db_type: "mysql", port: 3306 }); + + globalThis.fetch = withConnectionHealthMock(async (input, init) => { + const url = String(input); + if (url === "/api/query/execute-multi") { + return new Response( + JSON.stringify([ + { + columns: ["id", "sno", "batchs_id", "simple_id", "comments"], + rows: [[2658055, 4, 57485, 492045, null]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url === "/api/query/analyze-editability") { + const body = JSON.parse(String(init?.body ?? "{}")); + assert.equal(body.sql, sql); + return new Response(JSON.stringify(analyzeEditableQueryEditability(body.sql)), { status: 200, headers: { "Content-Type": "application/json" } }); + } + if (url.startsWith("/api/schema/columns?")) { + const table = new URL(url, "http://localhost").searchParams.get("table") ?? ""; + columnRequests.push(table); + const columnsByTable: Record>> = { + lims_simple: [ + { name: "id", data_type: "bigint", is_nullable: false, column_default: null, is_primary_key: true, extra: null, comment: null }, + { name: "sample_name", data_type: "text", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + ], + lims_batchs_simple: [ + { name: "id", data_type: "bigint", is_nullable: false, column_default: null, is_primary_key: true, extra: null, comment: null }, + { name: "sno", data_type: "integer", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "batchs_id", data_type: "bigint", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "simple_id", data_type: "bigint", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "comments", data_type: "text", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + ], + lims_batchs: [ + { name: "id", data_type: "bigint", is_nullable: false, column_default: null, is_primary_key: true, extra: null, comment: null }, + { name: "sno", data_type: "text", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + ], + }; + return new Response(JSON.stringify(columnsByTable[table] ?? []), { status: 200, headers: { "Content-Type": "application/json" } }); + } + if (url === "/api/query/prepare-pagination-plan") { + const body = JSON.parse(String(init?.body ?? "{}")); + return new Response(JSON.stringify({ sqlToExecute: body.options.sql, useAgentResultSession: false }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response("unexpected request", { status: 500 }); + }); + + try { + const tabId = store.createTab("mysql-join-delete", "lims", "Query 1", "query"); + await store.executeTabSql(tabId, sql); + + const tab = store.tabs.find((item) => item.id === tabId); + await waitFor(() => columnRequests.length === 3 && tab?.tableMeta?.tableName === "lims_batchs_simple"); + assert.deepEqual(columnRequests, ["lims_simple", "lims_batchs_simple", "lims_batchs"]); + assert.equal(tab?.queryEditabilityReason, undefined); + assert.equal(tab?.queryAnalysis?.multiSource, true); + assert.equal(tab?.queryAnalysis?.allowInsert, undefined); + assert.equal(tab?.queryAnalysis?.allowDelete, true); + assert.equal(tab?.queryAnalysis?.allowInsertDelete, false); + assert.equal(tab?.tableMeta?.tableName, "lims_batchs_simple"); + assert.deepEqual(tab?.tableMeta?.primaryKeys, ["id"]); + assert.deepEqual(tab?.querySourceColumns, ["id", "sno", "batchs_id", "simple_id", "comments"]); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } +}); + +test("keeps joined query rows read-only when the only candidate source has no primary keys", async () => { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + const columnRequests: string[] = []; + const sql = `SELECT e.* FROM audit_events e +INNER JOIN audit_sources s ON s.id = e.source_id +WHERE s.code = 'SRC-1'`; + + connectionStore.addEphemeralConnection(conn("pg-join-keyless")); + + globalThis.fetch = withConnectionHealthMock(async (input, init) => { + const url = String(input); + if (url === "/api/query/execute-multi") { + return new Response( + JSON.stringify([ + { + columns: ["event_no", "payload", "source_id"], + rows: [["EVT-9", "reload", 4]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url === "/api/query/analyze-editability") { + const body = JSON.parse(String(init?.body ?? "{}")); + assert.equal(body.sql, sql); + return new Response(JSON.stringify(analyzeEditableQueryEditability(body.sql)), { status: 200, headers: { "Content-Type": "application/json" } }); + } + if (url.startsWith("/api/schema/columns?")) { + const table = new URL(url, "http://localhost").searchParams.get("table") ?? ""; + columnRequests.push(table); + const columnsByTable: Record>> = { + audit_events: [ + { name: "event_no", data_type: "varchar", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "payload", data_type: "text", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "source_id", data_type: "integer", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + ], + audit_sources: [ + { name: "id", data_type: "integer", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + { name: "code", data_type: "varchar", is_nullable: false, column_default: null, is_primary_key: false, extra: null, comment: null }, + ], + }; + return new Response(JSON.stringify(columnsByTable[table] ?? []), { status: 200, headers: { "Content-Type": "application/json" } }); + } + if (url === "/api/query/prepare-pagination-plan") { + const body = JSON.parse(String(init?.body ?? "{}")); + return new Response(JSON.stringify({ sqlToExecute: body.options.sql, useAgentResultSession: false }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response("unexpected request", { status: 500 }); + }); + + try { + const tabId = store.createTab("pg-join-keyless", "appdb", "Query 1", "query"); + await store.executeTabSql(tabId, sql); + + const tab = store.tabs.find((item) => item.id === tabId); + await waitFor(() => columnRequests.length === 2 && tab?.queryEditabilityReason === "no-primary-key"); + assert.deepEqual(columnRequests, ["audit_events", "audit_sources"]); + assert.equal(tab?.queryAnalysis, undefined); + assert.equal(tab?.tableMeta, undefined); + assert.equal(tab?.querySourceColumns, undefined); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } +}); + test("binds DISTINCT qualified-star edits to the single safe joined source", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); @@ -3276,6 +3438,7 @@ test("binds DISTINCT qualified-star edits to the single safe joined source", asy assert.equal(tab?.queryAnalysis?.multiSource, true); assert.equal(tab?.queryAnalysis?.distinct, true); assert.equal(tab?.queryAnalysis?.allowInsert, true); + assert.equal(tab?.queryAnalysis?.allowDelete, false); assert.equal(tab?.queryAnalysis?.allowInsertDelete, false); assert.equal(tab?.tableMeta?.tableName, "users"); assert.deepEqual(tab?.querySourceColumns, ["id", "name"]);