diff --git a/apps/desktop/src/components/layout/ContentArea.vue b/apps/desktop/src/components/layout/ContentArea.vue index 8cd90eecac..8d4396e336 100644 --- a/apps/desktop/src/components/layout/ContentArea.vue +++ b/apps/desktop/src/components/layout/ContentArea.vue @@ -1569,7 +1569,7 @@ defineExpose({ focusSearch, refreshData, refreshQueryEditorCompletionCache, hand :custom-save-handler="mongoQueryResultSaveHandler" :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 !== false && activeTab.queryAnalysis?.allowInsertDelete !== false" + :allow-insert-rows="activeTab.queryAnalysis?.allowInsert ?? activeTab.queryAnalysis?.allowInsertDelete !== false" :allow-delete-rows="activeTab.queryAnalysis?.allowInsertDelete !== false" context="results" :auto-transpose-single-row="settingsStore.editorSettings.dataGridAutoTransposeSingleRow" diff --git a/apps/desktop/src/lib/sql/sqlAnalysis.ts b/apps/desktop/src/lib/sql/sqlAnalysis.ts index c6afbe4234..5ef884aeaa 100644 --- a/apps/desktop/src/lib/sql/sqlAnalysis.ts +++ b/apps/desktop/src/lib/sql/sqlAnalysis.ts @@ -160,7 +160,7 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability { tableAlias: source.alias, selectStar, columns, - ...(distinct ? { distinct: true, allowInsertDelete: false } : {}), + ...(distinct ? { distinct: true, allowInsert: false, allowInsertDelete: false } : {}), }; if (sources.length > 1) { analysis.sources = sources; diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index a504156cf3..b8852f79b4 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -35,7 +35,7 @@ import { nextRedisCommandDb } from "@/lib/redis/redisCommandSession"; import { isRedisMutatingCommand } from "@/lib/redis/redisCommandTable"; import { usesAgentCursorForQuery } from "@/lib/database/databaseDriverManifest"; import { supportsClearableQuerySchema } from "@/lib/database/databaseFeatureSupport"; -import { canUseKeylessRowPredicate, DBX_ROWID_COLUMN, editablePrimaryKeys, usesSyntheticRowIdKey } from "@/lib/table/tableEditing"; +import { canInsertTableRows, canUseKeylessRowPredicate, DBX_ROWID_COLUMN, editablePrimaryKeys, usesSyntheticRowIdKey } from "@/lib/table/tableEditing"; import { TABLE_DATA_EXPORT_PAGE_SIZE } from "@/lib/table/tableDataExport"; import { tableMetaForDataTab } from "@/lib/table/tableDataTabMeta"; import { dataTabExecutionDatabase } from "@/lib/table/dataTabExecutionDatabase"; @@ -3380,6 +3380,16 @@ export const useQueryStore = defineStore("query", () => { writeSchema?: string; }; + function canInsertIntoEditableQuerySource(tab: QueryTab, databaseType: DatabaseType | undefined, loaded: LoadedEditableSource, sourceColumns: readonly (string | undefined)[] | undefined): boolean { + if (!canInsertTableRows(databaseType) || !sourceColumns?.length || !sourceColumns.every(Boolean)) return false; + const knownTableType = + loaded.tableMeta.tableType ?? + useConnectionStore() + .lookupLocalCompletionTables(tab.connectionId!, loaded.tableMeta.database ?? tab.database, loaded.tableMeta.tableName, 20, loaded.tableMeta.schema, loaded.tableMeta.catalog) + .find((table) => table.name.toLowerCase() === loaded.tableMeta.tableName.toLowerCase())?.type; + return !knownTableType?.toUpperCase().includes("VIEW"); + } + interface EditableQueryExecutionPreparation { sql: string; metadataSql: string; @@ -3699,7 +3709,7 @@ export const useQueryStore = defineStore("query", () => { } return { - queryAnalysis: metadataAnalysis, + queryAnalysis: metadataAnalysis.distinct && canInsertIntoEditableQuerySource(tab, dbType as DatabaseType, loaded, sourceColumns) ? { ...metadataAnalysis, allowInsert: true } : metadataAnalysis, querySourceColumns: sourceColumns, queryEditabilityReason: undefined, tableMeta: primaryKeys === loaded.tableMeta.primaryKeys ? loaded.tableMeta : { ...loaded.tableMeta, primaryKeys }, @@ -3727,6 +3737,7 @@ export const useQueryStore = defineStore("query", () => { const target = candidates[0]!; const queryAnalysis = { ...target.analysis, + ...(target.analysis.distinct && canInsertIntoEditableQuerySource(tab, dbType as DatabaseType, target, target.sourceColumns) ? { allowInsert: true } : {}), allowInsertDelete: false, multiSource: true, }; diff --git a/apps/desktop/src/types/database.ts b/apps/desktop/src/types/database.ts index 60be540885..1b83724955 100644 --- a/apps/desktop/src/types/database.ts +++ b/apps/desktop/src/types/database.ts @@ -1247,6 +1247,7 @@ export interface QueryTab { multiSource?: boolean; allowInsert?: boolean; allowInsertDelete?: boolean; + distinct?: boolean; sources?: { key: string; catalog?: string; diff --git a/crates/dbx-core/src/sql_editability.rs b/crates/dbx-core/src/sql_editability.rs index 969cf938c2..b26713956d 100644 --- a/crates/dbx-core/src/sql_editability.rs +++ b/crates/dbx-core/src/sql_editability.rs @@ -23,6 +23,8 @@ pub struct EditableQueryInfo { #[serde(default, skip_serializing_if = "is_false")] pub multi_source: bool, #[serde(default, skip_serializing_if = "Option::is_none")] + pub allow_insert: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub allow_insert_delete: Option, #[serde(default, skip_serializing_if = "is_false")] pub distinct: bool, @@ -215,6 +217,7 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability { multi_source: false, // Updating an identified base row is safe, but insert/delete semantics are // ambiguous when the displayed set is de-duplicated by the query. + allow_insert: distinct.then_some(false), allow_insert_delete: distinct.then_some(false), distinct, }; @@ -1230,6 +1233,7 @@ mod tests { assert!(result.editable); let analysis = result.analysis.unwrap(); assert!(analysis.distinct); + assert_eq!(analysis.allow_insert, Some(false)); assert_eq!(analysis.allow_insert_delete, Some(false)); assert_eq!(analysis.columns.len(), 2); } @@ -1237,15 +1241,27 @@ mod tests { #[test] fn recognizes_distinct_qualified_star_from_join() { let result = analyze_editable_query_editability( - "select distinct u.* from users u left join orders o on o.user_id = u.id", + "select distinct t4.* + from TASK_CHECK_BASE t1 + left join TASK_FORMULATE_EXECUTE t20 on t1.EXECUTE_UID = t20.EXECUTE_UID + left join TASK_FORMULATE_BASE t40 on t20.FORMULATE_UID = t40.FORMULATE_UID + left join TASK_EXECUTE_FORM t30 on t20.EXECUTE_UID = t30.EXECUTE_UID + left join TASK_CHECK_ORG t2 on t1.TASK_ID = t2.TASK_ID + left join TASK_CHECK_ORG_INNER_DEPT t3 on t2.TASK_ORG_ID = t3.TASK_ORG_ID + left join TASK_CHECK_ENT t4 on t1.TASK_ID = t4.TASK_ID + left join TASK_EXECUTE_OBJ_ENT t44 on t1.EXECUTE_UID = t44.EXECUTE_UID + left join TASK_CHECK_DEPT_RESULT t5 on t4.TASK_ID = t5.TASK_ID and t4.TASK_ENT_ID = t5.TASK_ENT_ID + left join TASK_EXECUTE_PERSON_AGENT t6 on t1.EXECUTE_UID = t6.EXECUTE_UID", ); assert!(result.editable); let analysis = result.analysis.unwrap(); assert!(analysis.distinct); assert!(analysis.multi_source); + assert_eq!(analysis.allow_insert, Some(false)); assert_eq!(analysis.allow_insert_delete, Some(false)); - assert_eq!(analysis.columns, vec![star_column(Some("u"), Some("u:0"), "u.*")]); + assert_eq!(analysis.sources.as_ref().map(Vec::len), Some(10)); + assert_eq!(analysis.columns, vec![star_column(Some("t4"), Some("t4:6"), "t4.*")]); } #[test] diff --git a/packages/app-tests/dataGridEditor.test.ts b/packages/app-tests/dataGridEditor.test.ts index 3536117947..5715c82e74 100644 --- a/packages/app-tests/dataGridEditor.test.ts +++ b/packages/app-tests/dataGridEditor.test.ts @@ -52,8 +52,10 @@ function mockPreparedSaveStatements(options: DataGridSaveStatementOptions): stri statements.push(`UPDATE ${table} SET ${sets} WHERE ${primaryKeyWhere(options, row)};`); } for (const row of options.newRows) { - const columns = options.columns.map(quotePgIdentifier).join(", "); - const values = row.map((value) => formatGridSqlLiteral(value, options.databaseType)).join(", "); + const sourceColumns = options.sourceColumns?.length === options.columns.length ? options.sourceColumns : options.columns; + const insertColumns = sourceColumns.flatMap((column, index) => (column ? [{ column, index }] : [])); + const columns = insertColumns.map(({ column }) => quotePgIdentifier(column)).join(", "); + const values = insertColumns.map(({ index }) => formatGridSqlLiteral(row[index], options.databaseType)).join(", "); statements.push(`INSERT INTO ${table} (${columns}) VALUES (${values});`); } return statements; @@ -658,6 +660,50 @@ test("saving inserted rows reloads current table data", async () => { assert.deepEqual(emitted, [["reload", "SELECT id, name FROM people", "linus", "name ILIKE '%l%'", "id DESC", 50, 50]]); }); +test("saving a DISTINCT joined-source insert targets only the bound Dameng table columns", async () => { + setActivePinia(createPinia()); + installBrowserTestGlobals(); + + const result = computed(() => ({ + columns: ["TASK_ID", "TASK_ENT_ID", "NAME", "ROW_LABEL"], + rows: [[1, 10, "before", "computed"] as CellValue[]], + })); + const executedSql: string[] = []; + const editor = useDataGridEditor({ + result, + editable: computed(() => true), + databaseType: computed(() => "dameng"), + connectionId: computed(() => undefined), + database: computed(() => undefined), + tableMeta: computed(() => ({ + schema: "SYSDBA", + tableName: "TASK_CHECK_ENT", + columns: [column("TASK_ID", true), column("TASK_ENT_ID"), column("NAME")], + primaryKeys: ["TASK_ID"], + })), + sourceColumns: computed(() => ["TASK_ID", "TASK_ENT_ID", "NAME", undefined]), + onExecuteSql: computed(() => async (sql: string) => { + executedSql.push(sql); + }), + customSaveHandler: computed(() => undefined), + sql: computed(() => "SELECT DISTINCT t4.* FROM TASK_CHECK_BASE t1 LEFT JOIN TASK_CHECK_ENT t4 ON t1.TASK_ID = t4.TASK_ID"), + searchText: ref(""), + whereFilterInput: ref(""), + orderByInput: ref(""), + currentWhereInput: computed(() => undefined), + rowStatusFilter: ref<"all" | "changed" | "edited" | "new" | "deleted">("all"), + pageSize: ref(50), + currentPage: ref(1), + getRowItem: () => undefined, + emit: () => {}, + }); + + editor.newRows.value = [[2, 20, "grid-insert", "ignored expression"]]; + await editor.saveChanges(); + + assert.deepEqual(executedSql, [`INSERT INTO "SYSDBA"."TASK_CHECK_ENT" ("TASK_ID", "TASK_ENT_ID", "NAME") VALUES (2, 20, 'grid-insert');`]); +}); + test("saving edited rows without deletes does not reload table data", async () => { setActivePinia(createPinia()); installBrowserTestGlobals(); diff --git a/packages/app-tests/queryResultToolbar.test.ts b/packages/app-tests/queryResultToolbar.test.ts index f5aac7ed9d..1abe37c3a0 100644 --- a/packages/app-tests/queryResultToolbar.test.ts +++ b/packages/app-tests/queryResultToolbar.test.ts @@ -28,6 +28,13 @@ 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", () => { + 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"/); +}); + test("query result toolbar reuses the production icon contract", () => { const contentArea = source(contentAreaPath); const viewSwitcher = source(viewSwitcherPath); diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index 7c531ea1e5..cb3d962ba5 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -2196,6 +2196,7 @@ test("keeps PostgreSQL quoted primary keys distinct from case-only result column await waitFor(() => tab?.tableMeta?.tableName === "case_keys"); assert.match(executedSql, /"ID" AS "__DBX_PK_0"/); assert.deepEqual(tab?.querySourceColumns, ["id", "name", "ID"]); + assert.equal(tab?.queryAnalysis?.allowInsert, false); assert.equal(tab?.queryEditabilityReason, undefined); } finally { globalThis.fetch = originalFetch; @@ -2295,6 +2296,7 @@ test("binds DISTINCT qualified-star edits to the single safe joined source", asy assert.equal(tab?.queryEditabilityReason, undefined); assert.equal(tab?.queryAnalysis?.multiSource, true); assert.equal(tab?.queryAnalysis?.distinct, true); + assert.equal(tab?.queryAnalysis?.allowInsert, true); assert.equal(tab?.queryAnalysis?.allowInsertDelete, false); assert.equal(tab?.tableMeta?.tableName, "users"); assert.deepEqual(tab?.querySourceColumns, ["id", "name"]); @@ -2304,6 +2306,95 @@ test("binds DISTINCT qualified-star edits to the single safe joined source", asy } }); +test("enables DISTINCT inserts only for an insertable base-table target", async () => { + for (const scenario of [ + { id: "table", databaseType: "postgres", tableType: "TABLE" }, + { id: "view", databaseType: "postgres", tableType: "VIEW" }, + { id: "unsupported", databaseType: "jdbc", tableType: "TABLE" }, + { id: "computed", databaseType: "postgres", tableType: "TABLE" }, + ] as const) { + const restoreStorage = installMemoryStorage(); + setActivePinia(createPinia()); + const connectionStore = useConnectionStore(); + const store = useQueryStore(); + const originalFetch = globalThis.fetch; + connectionStore.addEphemeralConnection({ ...conn(`distinct-${scenario.id}`), db_type: scenario.databaseType }); + connectionStore.treeNodes.push({ + id: `${scenario.id}-users`, + label: "users", + type: scenario.tableType === "VIEW" ? "view" : "table", + connectionId: `distinct-${scenario.id}`, + database: "appdb", + schema: "public", + tableName: "users", + }); + + globalThis.fetch = withConnectionHealthMock(async (input, init) => { + const url = String(input); + if (url === "/api/query/execute-multi") { + return new Response(JSON.stringify([{ columns: scenario.id === "computed" ? ["id", "label"] : ["id", "name"], rows: [[1, "Ada"]], affected_rows: 0, execution_time_ms: 1 }]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + if (url === "/api/query/analyze-editability") { + return new Response( + JSON.stringify({ + editable: true, + analysis: { + schema: undefined, + schemaQuoted: false, + tableName: "users", + tableNameQuoted: false, + selectStar: false, + distinct: true, + allowInsert: false, + allowInsertDelete: false, + columns: + scenario.id === "computed" + ? [ + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { resultName: "label", expression: "upper(name) as label" }, + ] + : [ + { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, + { sourceName: "name", sourceNameQuoted: false, resultName: "name", expression: "name" }, + ], + }, + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + if (url.startsWith("/api/schema/columns?")) { + return new Response( + JSON.stringify([ + { name: "id", data_type: "integer", is_nullable: false, column_default: null, is_primary_key: true, extra: null, comment: null }, + { name: "name", data_type: "text", is_nullable: true, column_default: null, is_primary_key: false, extra: null, comment: null }, + ]), + { 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(`distinct-${scenario.id}`, "appdb", "Query 1", "query", "public"); + const tab = store.tabs.find((item) => item.id === tabId)!; + await store.executeTabSql(tabId, scenario.id === "computed" ? "select distinct id, upper(name) as label from users" : "select distinct id, name from users"); + await waitFor(() => tab.tableMeta?.columns.length === 2); + assert.equal(tab.queryAnalysis?.allowInsert, scenario.id === "table", scenario.id); + assert.equal(tab.queryAnalysis?.allowInsertDelete, false, scenario.id); + } finally { + globalThis.fetch = originalFetch; + restoreStorage(); + } + } +}); + test("expands single-table alias star projections for editable query metadata", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); diff --git a/packages/app-tests/sqlAnalysis.test.ts b/packages/app-tests/sqlAnalysis.test.ts index 68803b8680..d1b6e06a33 100644 --- a/packages/app-tests/sqlAnalysis.test.ts +++ b/packages/app-tests/sqlAnalysis.test.ts @@ -132,6 +132,7 @@ test("treats DISTINCT single-table projections as update-only", () => { assert.equal(result.editable, true); assert.equal(result.analysis.distinct, true); + assert.equal(result.analysis.allowInsert, false); assert.equal(result.analysis.allowInsertDelete, false); assert.deepEqual(result.analysis.columns, [ { sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" }, @@ -140,13 +141,25 @@ test("treats DISTINCT single-table projections as update-only", () => { }); test("maps a DISTINCT qualified star to one joined source", () => { - const result = analyzeEditableQueryEditability("select distinct u.* from users u left join orders o on o.user_id = u.id"); + const result = analyzeEditableQueryEditability(`select distinct t4.* + from TASK_CHECK_BASE t1 + left join TASK_FORMULATE_EXECUTE t20 on t1.EXECUTE_UID = t20.EXECUTE_UID + left join TASK_FORMULATE_BASE t40 on t20.FORMULATE_UID = t40.FORMULATE_UID + left join TASK_EXECUTE_FORM t30 on t20.EXECUTE_UID = t30.EXECUTE_UID + left join TASK_CHECK_ORG t2 on t1.TASK_ID = t2.TASK_ID + left join TASK_CHECK_ORG_INNER_DEPT t3 on t2.TASK_ORG_ID = t3.TASK_ORG_ID + left join TASK_CHECK_ENT t4 on t1.TASK_ID = t4.TASK_ID + left join TASK_EXECUTE_OBJ_ENT t44 on t1.EXECUTE_UID = t44.EXECUTE_UID + left join TASK_CHECK_DEPT_RESULT t5 on t4.TASK_ID = t5.TASK_ID and t4.TASK_ENT_ID = t5.TASK_ENT_ID + left join TASK_EXECUTE_PERSON_AGENT t6 on t1.EXECUTE_UID = t6.EXECUTE_UID`); assert.equal(result.editable, true); assert.equal(result.analysis.distinct, true); assert.equal(result.analysis.multiSource, true); + assert.equal(result.analysis.allowInsert, false); assert.equal(result.analysis.allowInsertDelete, false); - assert.deepEqual(result.analysis.columns, [{ star: true, sourceQualifier: "u", sourceKey: "u:0", resultName: "*", expression: "u.*" }]); + assert.equal(result.analysis.sources?.length, 10); + assert.deepEqual(result.analysis.columns, [{ star: true, sourceQualifier: "t4", sourceKey: "t4:6", resultName: "*", expression: "t4.*" }]); }); test("keeps ambiguous DISTINCT projections read-only", () => {